Skip to content

Commit

Permalink
Merge branch 'master' into eschnett/float16-implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
eschnett committed Nov 1, 2024
2 parents eafee05 + a4505b9 commit 43e1189
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 41 deletions.
8 changes: 8 additions & 0 deletions MIGRATION_GUIDE.TXT
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
MIGRATION GUIDE FROM GDAL 3.9 to GDAL 3.10
------------------------------------------

- The OGR SQL parser has been modified to evaluate NULL values in boolean
operations similarly to other SQL engines (SQLite, PostgreSQL, etc.). Previously,
with a foo=NULL field, expressions ``foo NOT IN ('bar')`` and ``foo NOT LIKE ('bar')``
would evaluate as true. Now the result is false (with the NULL state being
propagated to it). Concretely, to get the same results as in previous versions,
the above expressions must be rewritten as ``foo IS NULL OR foo NOT IN ('bar')``
and ``foo IS NULL OR foo NOT LIKE ('bar')``.

- MEM driver: opening a dataset with the MEM::: syntax is now disabled by
default because of security implications. This can be enabled by setting the
GDAL_MEM_ENABLE_OPEN build or configuration option. Creation of a 0-band MEM
Expand Down
9 changes: 4 additions & 5 deletions autotest/cpp/test_ogr_swq.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,10 @@ class PushNotOperationDownToStackFixture
std::make_tuple("NOT(1 <= 2)", "1 > 2"),
std::make_tuple("NOT(1 < 2)", "1 >= 2"),
std::make_tuple("NOT(NOT(1))", "1"),
std::make_tuple("NOT(1 AND 2)", "(NOT (1)) OR (NOT (2))"),
std::make_tuple("NOT(1 OR 2)", "(NOT (1)) AND (NOT (2))"),
std::make_tuple("3 AND NOT(1 OR 2)",
"3 AND ((NOT (1)) AND (NOT (2)))"),
std::make_tuple("NOT(NOT(1 = 2) OR 2)", "(1 = 2) AND (NOT (2))"),
std::make_tuple("NOT(1 AND 2)", "(NOT 1) OR (NOT 2)"),
std::make_tuple("NOT(1 OR 2)", "(NOT 1) AND (NOT 2)"),
std::make_tuple("3 AND NOT(1 OR 2)", "3 AND ((NOT 1) AND (NOT 2))"),
std::make_tuple("NOT(NOT(1 = 2) OR 2)", "(1 = 2) AND (NOT 2)"),
std::make_tuple("1", "1"),
};
}
Expand Down
7 changes: 7 additions & 0 deletions autotest/ogr/ogr_sql_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,13 @@ def get_available_dialects():
("(NOT intfield = 0) AND NOT (intfield IS NULL)", 1),
("NOT (intfield = 0 OR intfield IS NOT NULL)", 0),
("(NOT intfield = 0) AND NOT (intfield IS NOT NULL)", 0),
("intfield <> 0 AND intfield <> 2", 1),
("intfield IS NOT NULL AND intfield NOT IN (2)", 1),
("NOT(intfield NOT IN (1) AND NULL NOT IN (1))", 1),
("NOT(intfield IS NOT NULL AND intfield NOT IN (2))", 1),
("NOT(NOT(intfield IS NOT NULL AND intfield NOT IN (2)))", 1),
("NOT (intfield = 0 AND intfield = 0)", 1),
("(intfield NOT IN (1) AND NULL NOT IN (1)) IS NULL", 1),
# realfield
("1 + realfield >= 0", 1),
("realfield = 0", 0),
Expand Down
72 changes: 37 additions & 35 deletions ogr/swq_expr_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,21 @@ CPLString swq_expr_node::UnparseOperationFromUnparsedSubExpr(char **apszSubExpr)
return osExpr;
}

const auto AddSubExpr = [this, apszSubExpr, &osExpr](int idx)
{
if (papoSubExpr[idx]->eNodeType == SNT_COLUMN ||
papoSubExpr[idx]->eNodeType == SNT_CONSTANT)
{
osExpr += apszSubExpr[idx];
}
else
{
osExpr += '(';
osExpr += apszSubExpr[idx];
osExpr += ')';
}
};

switch (nOperation)
{
// Binary infix operators.
Expand All @@ -683,63 +698,52 @@ CPLString swq_expr_node::UnparseOperationFromUnparsedSubExpr(char **apszSubExpr)
case SWQ_DIVIDE:
case SWQ_MODULUS:
CPLAssert(nSubExprCount >= 2);
if (papoSubExpr[0]->eNodeType == SNT_COLUMN ||
papoSubExpr[0]->eNodeType == SNT_CONSTANT)
{
osExpr += apszSubExpr[0];
}
else
{
osExpr += "(";
osExpr += apszSubExpr[0];
osExpr += ")";
}
AddSubExpr(0);
osExpr += " ";
osExpr += poOp->pszName;
osExpr += " ";
if (papoSubExpr[1]->eNodeType == SNT_COLUMN ||
papoSubExpr[1]->eNodeType == SNT_CONSTANT)
{
osExpr += apszSubExpr[1];
}
else
{
osExpr += "(";
osExpr += apszSubExpr[1];
osExpr += ")";
}
AddSubExpr(1);
if ((nOperation == SWQ_LIKE || nOperation == SWQ_ILIKE) &&
nSubExprCount == 3)
osExpr += CPLSPrintf(" ESCAPE (%s)", apszSubExpr[2]);
{
osExpr += " ESCAPE ";
AddSubExpr(2);
}
break;

case SWQ_NOT:
CPLAssert(nSubExprCount == 1);
osExpr.Printf("NOT (%s)", apszSubExpr[0]);
osExpr = "NOT ";
AddSubExpr(0);
break;

case SWQ_ISNULL:
CPLAssert(nSubExprCount == 1);
osExpr.Printf("%s IS NULL", apszSubExpr[0]);
AddSubExpr(0);
osExpr += " IS NULL";
break;

case SWQ_IN:
osExpr.Printf("%s IN (", apszSubExpr[0]);
AddSubExpr(0);
osExpr += " IN(";
for (int i = 1; i < nSubExprCount; i++)
{
if (i > 1)
osExpr += ",";
osExpr += "(";
osExpr += apszSubExpr[i];
osExpr += ")";
AddSubExpr(i);
}
osExpr += ")";
break;

case SWQ_BETWEEN:
CPLAssert(nSubExprCount == 3);
osExpr.Printf("%s %s (%s) AND (%s)", apszSubExpr[0], poOp->pszName,
apszSubExpr[1], apszSubExpr[2]);
AddSubExpr(0);
osExpr += ' ';
osExpr += poOp->pszName;
osExpr += ' ';
AddSubExpr(1);
osExpr += " AND ";
AddSubExpr(2);
break;

case SWQ_CAST:
Expand All @@ -760,7 +764,7 @@ CPLString swq_expr_node::UnparseOperationFromUnparsedSubExpr(char **apszSubExpr)
osExpr += apszSubExpr[i] + 1;
}
else
osExpr += apszSubExpr[i];
AddSubExpr(i);

if (i == 1 && nSubExprCount > 2)
osExpr += "(";
Expand All @@ -779,9 +783,7 @@ CPLString swq_expr_node::UnparseOperationFromUnparsedSubExpr(char **apszSubExpr)
{
if (i > 0)
osExpr += ",";
osExpr += "(";
osExpr += apszSubExpr[i];
osExpr += ")";
AddSubExpr(i);
}
osExpr += ")";
break;
Expand Down
7 changes: 6 additions & 1 deletion ogr/swq_op_general.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ swq_expr_node *SWQGeneralEvaluator(swq_expr_node *node,
poRet->field_type = node->field_type;

if (node->nOperation != SWQ_ISNULL && node->nOperation != SWQ_OR &&
node->nOperation != SWQ_AND && node->nOperation != SWQ_NOT &&
node->nOperation != SWQ_IN)
{
for (int i = 0; i < node->nSubExprCount; i++)
Expand All @@ -543,6 +544,8 @@ swq_expr_node *SWQGeneralEvaluator(swq_expr_node *node,
case SWQ_AND:
poRet->int_value = sub_node_values[0]->int_value &&
sub_node_values[1]->int_value;
poRet->is_null =
sub_node_values[0]->is_null && sub_node_values[1]->is_null;
break;

case SWQ_OR:
Expand All @@ -553,7 +556,9 @@ swq_expr_node *SWQGeneralEvaluator(swq_expr_node *node,
break;

case SWQ_NOT:
poRet->int_value = !sub_node_values[0]->int_value;
poRet->int_value = !sub_node_values[0]->int_value &&
!sub_node_values[0]->is_null;
poRet->is_null = sub_node_values[0]->is_null;
break;

case SWQ_EQ:
Expand Down

0 comments on commit 43e1189

Please sign in to comment.