GeoRaster: Added GENSTATS options, security fixes, and prevent failing when password is near expiration #9290
Conversation
GeoRaster: Added generate statistics option for GeoRaster driver
Statistics and general use fix
GeoRaster: fixed docs for GENSTATS_SAMPLINGWINDOW and GENSTATS_BINFUNCTION
…caped quotes instead of \'
initialized dfGenStatsBinFunction dfGenStatsSamplingWindow
added table prefix to columns in georaster_wrapper when generating statistics added code to ignore error ORA-28002
add documentation about GENSTATS must be set to true even if other options are set to false
added warning log when GENSTATS options are set but GENSTATS is false
|
@fechen123 Are you still involved with the GeoRaster driver? If so, can you give this some review? |
|
Some XML invalidity found in the creation options: |
Yes, Alan is helping us to add some features. I have reviewed the changes and they look good to me. |
| pszFetched = CSLFetchNameValue(papszOptions, "GENSTATS"); | ||
|
|
||
| if (pszFetched != nullptr) | ||
| { | ||
| poGRD->poGeoRaster->bGenStats = EQUAL(pszFetched, "TRUE"); | ||
| } |
There was a problem hiding this comment.
can be simplified as:
| pszFetched = CSLFetchNameValue(papszOptions, "GENSTATS"); | |
| if (pszFetched != nullptr) | |
| { | |
| poGRD->poGeoRaster->bGenStats = EQUAL(pszFetched, "TRUE"); | |
| } | |
| poGRD->poGeoRaster->bGenStats = CPLFetchBool(papszOptions, "GENSTATS", FALSE); |
similar occurrences below
| const char *rgpszInvalidChars[] = {";", "--", "/*", "*/", "//"}; | ||
| const size_t nInvalidCharsSize = 5; | ||
|
|
||
| for (size_t nPos = 0; nPos < nInvalidCharsSize; ++nPos) | ||
| { | ||
| const char *pszInvalidChar = rgpszInvalidChars[nPos]; |
There was a problem hiding this comment.
can be simplified as:
| const char *rgpszInvalidChars[] = {";", "--", "/*", "*/", "//"}; | |
| const size_t nInvalidCharsSize = 5; | |
| for (size_t nPos = 0; nPos < nInvalidCharsSize; ++nPos) | |
| { | |
| const char *pszInvalidChar = rgpszInvalidChars[nPos]; | |
| const char *rgpszInvalidChars[] = {";", "--", "/*", "*/", "//"}; | |
| for (const char *pszInvalidChar: rgpszInvalidChars) | |
| { |
| @@ -100,6 +100,33 @@ void GEORDriverSetCommonMetadata(GDALDriver *poDriver) | |||
| " </Option>" | |||
| " <Option name='GENPYRLEVELS' type='int' description='Number of " | |||
| "pyramid level to generate'/>" | |||
| " <Option name='GENSTATS' type='boolean'" | |||
| "description='Generate statistics from the given rasters'" | |||
There was a problem hiding this comment.
I suspect missing spaces cause XML validation issues:
| "description='Generate statistics from the given rasters'" | |
| "description='Generate statistics from the given rasters' " |
There was a problem hiding this comment.
You're right, I might missed those when merging with the latest main changes, I'm going to make a commit with those fixes.
| " <Option name='GENSTATS' type='boolean'" | ||
| "description='Generate statistics from the given rasters'" | ||
| "default='FALSE' />" | ||
| " <Option name='GENSTATS_SAMPLINGFACTOR' type='int'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_SAMPLINGFACTOR' type='int'" | |
| " <Option name='GENSTATS_SAMPLINGFACTOR' type='int' " |
| " <Option name='GENSTATS_SAMPLINGFACTOR' type='int'" | ||
| "description='Number of cells skipped in both row and column " | ||
| "dimensions when " | ||
| "the statistics are computed'" |
There was a problem hiding this comment.
| "the statistics are computed'" | |
| "the statistics are computed' " |
| "dimensions when " | ||
| "the statistics are computed'" | ||
| "default='1' />" | ||
| " <Option name='GENSTATS_SAMPLINGWINDOW' type='string'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_SAMPLINGWINDOW' type='string'" | |
| " <Option name='GENSTATS_SAMPLINGWINDOW' type='string' " |
| " <Option name='GENSTATS_SAMPLINGWINDOW' type='string'" | ||
| "description='Coordinates (4 numbers) of a rectangular " | ||
| "window to be used to sample the raster when generating statistics' />" | ||
| " <Option name='GENSTATS_HISTOGRAM' type='boolean'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_HISTOGRAM' type='boolean'" | |
| " <Option name='GENSTATS_HISTOGRAM' type='boolean' " |
| "window to be used to sample the raster when generating statistics' />" | ||
| " <Option name='GENSTATS_HISTOGRAM' type='boolean'" | ||
| "description='Compute a histogram for the raster' default='FALSE' />" | ||
| " <Option name='GENSTATS_LAYERNUMBERS' type='string'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_LAYERNUMBERS' type='string'" | |
| " <Option name='GENSTATS_LAYERNUMBERS' type='string' " |
| " <Option name='GENSTATS_LAYERNUMBERS' type='string'" | ||
| "description='Layer numbers and/or ranges for which to compute " | ||
| "the statistics' />" | ||
| " <Option name='GENSTATS_USEBIN' type='boolean'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_USEBIN' type='boolean'" | |
| " <Option name='GENSTATS_USEBIN' type='boolean' " |
| "description='Specifies if the statistics should use the bin function " | ||
| "provided by GENSTATS_USEBIN " | ||
| "to compute the statistics' default='TRUE' />" | ||
| " <Option name='GENSTATS_BINFUNCTION' type='string'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_BINFUNCTION' type='string'" | |
| " <Option name='GENSTATS_BINFUNCTION' type='string' " |
| " <Option name='GENSTATS_BINFUNCTION' type='string'" | ||
| "description='Array to specify the bin function (type, total number of " | ||
| "bins, first bin number, minimum, cell value, maximum cell value)' />" | ||
| " <Option name='GENSTATS_NODATA' type='boolean'" |
There was a problem hiding this comment.
| " <Option name='GENSTATS_NODATA' type='boolean'" | |
| " <Option name='GENSTATS_NODATA' type='boolean' " |
simplified boolean value retrieval with CPLFetchBool simplified for loop iteration in ValidateDescriptionExpression
| bGenStatsOptionsUsed = true; | ||
| poGRD->poGeoRaster->bGenStatsHistogram = EQUAL(pszFetched, "TRUE"); | ||
| } | ||
| poGRD->poGeoRaster->bGenStatsHistogram = CPLFetchBool(papszOptions, "GENSTATS_HISTOGRAM", FALSE); |
There was a problem hiding this comment.
hum I realize my suggestion to use CPLFetchBool might cause issues here as bGenStatsOptionsUsed = true will no longer be set.
But I also realize that autotest/gdrivers/georaster.py hasn't been updated. It could make sense for long-term maintenance to test that new functionality
|
code formatting issues (https://github.com/OSGeo/gdal/actions/runs/8052255527/job/21996379266?pr=9290) and still XML invalidity |
|
see https://gdal.org/development/dev_practices.html#commit-hooks how to set up pre-commit to get auto formatting |
restore previous code in georaster_dataset.cpp format in ValidateDescriptionExpression change name in XML
|
formatting issue in the python test: https://github.com/OSGeo/gdal/actions/runs/8087419569/job/22099351834?pr=9290 |
|
There are still XML errors. |
No description provided.