Skip to content

Commit 409ec85

Browse files
authoredApr 24, 2018
[FEATURE][needs-docs] Rework expression parser for unknown functions (#6840)
* [FEATURE][needs-docs] Rework expression parser for unknown functions * Remove old comment * Rename NAME and update tests * Change COLUMN_REF to QUOTED_COLUMN_REF
1 parent 70b67c6 commit 409ec85

File tree

3 files changed

+33
-18
lines changed

3 files changed

+33
-18
lines changed
 

‎src/core/qgsexpressionlexer.ll

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,16 @@ non_ascii [\x80-\xFF]
128128

129129
col_first [A-Za-z_]|{non_ascii}
130130
col_next [A-Za-z0-9_]|{non_ascii}
131-
column_ref {col_first}{col_next}*
131+
identifier {col_first}{col_next}*
132132

133133
deprecated_function "$"[xXyY]_?[aA][tT]
134-
special_col "$"{column_ref}
135-
variable "@"{column_ref}
134+
special_col "$"{identifier}
135+
variable "@"{identifier}
136136

137-
named_node {column_ref}{white}*":="{white}*
137+
named_node {identifier}{white}*":="{white}*
138138

139139
col_str_char "\"\""|[^\"]
140-
column_ref_quoted "\""{col_str_char}*"\""
140+
identifier_quoted "\""{col_str_char}*"\""
141141
142142
dig [0-9]
143143
num_int {dig}+
@@ -220,17 +220,17 @@ string "'"{str_char}*"'"
220220
221221
{string} { TEXT_FILTER(stripText); return STRING; }
222222
223-
{deprecated_function} { TEXT; return FUNCTION; }
223+
{deprecated_function} { TEXT; return NAME; }
224224
225225
{named_node} { TEXT_FILTER(stripNamedText); return NAMED_NODE; }
226226
227227
{special_col} { TEXT; return SPECIAL_COL; }
228228
229229
{variable} { TEXT; return VARIABLE; }
230230
231-
{column_ref} { TEXT; return QgsExpression::isFunctionName(*yylval->text) ? FUNCTION : COLUMN_REF; }
231+
{identifier} { TEXT; return NAME; }
232232
233-
{column_ref_quoted} { TEXT_FILTER(stripColumnRef); return COLUMN_REF; }
233+
{identifier_quoted} { TEXT_FILTER(stripColumnRef); return QUOTED_COLUMN_REF; }
234234
235235
{white} /* skip blanks and tabs */
236236

‎src/core/qgsexpressionparser.yy

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ void addParserLocation(YYLTYPE* yyloc, QgsExpressionNode *node)
125125
// tokens for conditional expressions
126126
%token CASE WHEN THEN ELSE END
127127

128-
%token <text> STRING COLUMN_REF FUNCTION SPECIAL_COL VARIABLE NAMED_NODE
128+
%token <text> STRING QUOTED_COLUMN_REF NAME SPECIAL_COL VARIABLE NAMED_NODE
129129

130130
%token COMMA
131131

@@ -203,14 +203,12 @@ expression:
203203
| expression CONCAT expression { $$ = BINOP($2, $1, $3); }
204204
| NOT expression { $$ = new QgsExpressionNodeUnaryOperator($1, $2); }
205205
| '(' expression ')' { $$ = $2; }
206-
| FUNCTION '(' exp_list ')'
206+
| NAME '(' exp_list ')'
207207
{
208208
int fnIndex = QgsExpression::functionIndex(*$1);
209209
delete $1;
210210
if (fnIndex == -1)
211211
{
212-
// this should not actually happen because already in lexer we check whether an identifier is a known function
213-
// (if the name is not known the token is parsed as a column)
214212
QgsExpression::ParserError::ParserErrorType errorType = QgsExpression::ParserError::FunctionUnknown;
215213
parser_ctx->currentErrorType = errorType;
216214
exp_error(&yyloc, parser_ctx, "Function is not known");
@@ -240,14 +238,12 @@ expression:
240238
addParserLocation(&@1, $$);
241239
}
242240

243-
| FUNCTION '(' ')'
241+
| NAME '(' ')'
244242
{
245243
int fnIndex = QgsExpression::functionIndex(*$1);
246244
delete $1;
247245
if (fnIndex == -1)
248246
{
249-
// this should not actually happen because already in lexer we check whether an identifier is a known function
250-
// (if the name is not known the token is parsed as a column)
251247
QgsExpression::ParserError::ParserErrorType errorType = QgsExpression::ParserError::FunctionUnknown;
252248
parser_ctx->currentErrorType = errorType;
253249
exp_error(&yyloc, parser_ctx, "Function is not known");
@@ -276,7 +272,8 @@ expression:
276272
| CASE when_then_clauses ELSE expression END { $$ = new QgsExpressionNodeCondition($2,$4); }
277273

278274
// columns
279-
| COLUMN_REF { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }
275+
| NAME { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }
276+
| QUOTED_COLUMN_REF { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }
280277

281278
// special columns (actually functions with no arguments)
282279
| SPECIAL_COL

‎tests/src/core/testqgsexpression.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ class TestQgsExpression: public QObject
206206
QTest::newRow( "invalid character" ) << "@" << false;
207207
QTest::newRow( "invalid column reference" ) << "my col" << false;
208208
QTest::newRow( "invalid binary operator" ) << "1+" << false;
209-
QTest::newRow( "invalid function no params" ) << "cos" << false;
209+
QTest::newRow( "invalid function not known no args" ) << "watwat()" << false;
210210
QTest::newRow( "invalid function not known" ) << "coz(1)" << false;
211211
QTest::newRow( "invalid operator IN" ) << "n in p" << false;
212212
QTest::newRow( "empty node list" ) << "1 in ()" << false;
@@ -1362,10 +1362,12 @@ class TestQgsExpression: public QObject
13621362
fields.append( QgsField( QStringLiteral( "x1" ) ) );
13631363
fields.append( QgsField( QStringLiteral( "x2" ) ) );
13641364
fields.append( QgsField( QStringLiteral( "foo" ), QVariant::Int ) );
1365+
fields.append( QgsField( QStringLiteral( "sin" ), QVariant::Int ) );
13651366

13661367
QgsFeature f;
1367-
f.initAttributes( 3 );
1368+
f.initAttributes( 4 );
13681369
f.setAttribute( 2, QVariant( 20 ) );
1370+
f.setAttribute( 3, QVariant( 10 ) );
13691371

13701372
QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( f, fields );
13711373

@@ -1385,6 +1387,22 @@ class TestQgsExpression: public QObject
13851387
QCOMPARE( exp2.hasEvalError(), true );
13861388
QVariant res2 = exp2.evaluate( &context );
13871389
QCOMPARE( res2.type(), QVariant::Invalid );
1390+
1391+
// Has field called sin and function
1392+
QgsExpression exp3( QStringLiteral( "sin" ) );
1393+
prepareRes = exp3.prepare( &context );
1394+
QCOMPARE( prepareRes, true );
1395+
QCOMPARE( exp3.hasEvalError(), false );
1396+
res = exp3.evaluate( &context );
1397+
QCOMPARE( res.type(), QVariant::Int );
1398+
QCOMPARE( res.toInt(), 10 );
1399+
1400+
QgsExpression exp4( QStringLiteral( "sin(3.14)" ) );
1401+
prepareRes = exp4.prepare( &context );
1402+
QCOMPARE( prepareRes, true );
1403+
QCOMPARE( exp4.hasEvalError(), false );
1404+
res = exp4.evaluate( &context );
1405+
QCOMPARE( res.toInt(), 0 );
13881406
}
13891407

13901408
void eval_feature_id()

0 commit comments

Comments
 (0)
Please sign in to comment.