#4259 closed defect (fixed)
OGR SQL implicit conversion from string to numeric
Reported by: | tamas | Owned by: | tamas |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | default | Version: | |
Severity: | normal | Keywords: | |
Cc: |
Description
Most SQL query implementations support implicit conversions between various types in the SQL expressions. For example we can safely use the following syntax:
select * from rivers where ogr_fid in ('1100','1101','1102')
If the data type of ogr_fid is an integer or float then the expression evaluator would convert the string constants to numeric values implicitly.
This conversion has been working until the new expression parser have was introduced in gdal 1.8. As of 1.8 the parser reports an error due to the incompatible types in the expression.
Attachments (1)
Change History (15)
comment:1 by , 13 years ago
by , 13 years ago
Attachment: | swq_op_general.cpp.patch added |
---|
follow-up: 3 comment:2 by , 12 years ago
Tamas, your patch looks good and my testing didn't show any issue, so I think you can just go ahead and commit it.
One small question however: why is "poSubNode->int_value = (int)poSubNode->float_value;" necessary ? If the node is declared as SWQ_FLOAT, no one should ever try to read the int_value field. For example, the swq_expr_node::swq_expr_node( double dfValueIn ) constructor only set the float_value field.
comment:3 by , 12 years ago
Replying to rouault:
Tamas, your patch looks good and my testing didn't show any issue, so I think you can just go ahead and commit it.
One small question however: why is "poSubNode->int_value = (int)poSubNode->float_value;" necessary ? If the node is declared as SWQ_FLOAT, no one should ever try to read the int_value field. For example, the swq_expr_node::swq_expr_node( double dfValueIn ) constructor only set the float_value field.
Even, in some cases the field type is used instead of the node type (for an example see ogrfeaturequery.cpp line 296) In a particular case if ogr_fid is an integer field, we may have problems with the string to float conversion if we don't set int_value in addition.
follow-ups: 5 10 comment:4 by , 12 years ago
Interesting, but I believe you've just found a bug in OGRFeatureQuery::EvaluateAgainstIndices() that should be fixed (by testing the node type and doing the appropriate cast), and not in your new code. Because we currently have a bug *without* your new code :
ogr2ogr mypoly.shp ../autotest/ogr/data/poly.shp -sql "select cast(EAS_ID as integer) as EAS_ID_INT, * from poly" ogrinfo mypoly.shp -sql "create index on mypoly using EAS_ID_INT" ogrinfo mypoly.shp -where "EAS_ID_INT in (170)" -al --> works ogrinfo mypoly.shp -where "EAS_ID_INT in (170.0)" -al --> doesn't work
comment:5 by , 12 years ago
I think the safest way is to leave the "poSubNode->int_value = (int)poSubNode->float_value;" assignment as it is, and provide a fix for the bug in addition.
comment:7 by , 12 years ago
Applied in trunk (r23205).
I'm not sure whether to treat this one as a bug or new feature and include the change in the stable branch as well? Anyway this functionality have been working in the pre 1.8 versions before the new parser was added...
comment:8 by , 12 years ago
comment:9 by , 12 years ago
follow-up: 11 comment:10 by , 12 years ago
Replying to rouault:
Interesting, but I believe you've just found a bug in OGRFeatureQuery::EvaluateAgainstIndices() that should be fixed (by testing the node type and doing the appropriate cast), and not in your new code. Because we currently have a bug *without* your new code :
ogr2ogr mypoly.shp ../autotest/ogr/data/poly.shp -sql "select cast(EAS_ID as integer) as EAS_ID_INT, * from poly" ogrinfo mypoly.shp -sql "create index on mypoly using EAS_ID_INT" ogrinfo mypoly.shp -where "EAS_ID_INT in (170)" -al --> works ogrinfo mypoly.shp -where "EAS_ID_INT in (170.0)" -al --> doesn't work
Should we handle this problem within the scope of this issue?
comment:11 by , 12 years ago
Should we handle this problem within the scope of this issue?
That's probably worth a new ticket
comment:13 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I close this ticket. I've created a separate ticket for the side issue mentionned above (bug in EvaluateAgainstIndices()) : #4326
Patch have been added to provide a string to numeric conversion