#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 Changed 10 years ago by
Changed 10 years ago by
Attachment: | swq_op_general.cpp.patch added |
---|
comment:2 follow-up: 3 Changed 10 years ago by
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 Changed 10 years ago by
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.
comment:4 follow-ups: 5 10 Changed 10 years ago by
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 Changed 10 years ago by
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 Changed 10 years ago by
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 Changed 10 years ago by
comment:9 Changed 10 years ago by
comment:10 follow-up: 11 Changed 10 years ago by
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 Changed 10 years ago by
Should we handle this problem within the scope of this issue?
That's probably worth a new ticket
comment:12 Changed 9 years ago by
Somehow related ticket #4321 about joining columns of different types
comment:13 Changed 9 years ago by
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