Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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
  • Loading branch information
NathanW2 committed Apr 24, 2018
1 parent 70b67c6 commit 409ec85
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 18 deletions.
16 changes: 8 additions & 8 deletions src/core/qgsexpressionlexer.ll
Expand Up @@ -128,16 +128,16 @@ non_ascii [\x80-\xFF]

col_first [A-Za-z_]|{non_ascii}
col_next [A-Za-z0-9_]|{non_ascii}
column_ref {col_first}{col_next}*
identifier {col_first}{col_next}*

deprecated_function "$"[xXyY]_?[aA][tT]
special_col "$"{column_ref}
variable "@"{column_ref}
special_col "$"{identifier}
variable "@"{identifier}

named_node {column_ref}{white}*":="{white}*
named_node {identifier}{white}*":="{white}*

col_str_char "\"\""|[^\"]
column_ref_quoted "\""{col_str_char}*"\""
identifier_quoted "\""{col_str_char}*"\""
dig [0-9]
num_int {dig}+
Expand Down Expand Up @@ -220,17 +220,17 @@ string "'"{str_char}*"'"
{string} { TEXT_FILTER(stripText); return STRING; }
{deprecated_function} { TEXT; return FUNCTION; }
{deprecated_function} { TEXT; return NAME; }
{named_node} { TEXT_FILTER(stripNamedText); return NAMED_NODE; }
{special_col} { TEXT; return SPECIAL_COL; }
{variable} { TEXT; return VARIABLE; }
{column_ref} { TEXT; return QgsExpression::isFunctionName(*yylval->text) ? FUNCTION : COLUMN_REF; }
{identifier} { TEXT; return NAME; }
{column_ref_quoted} { TEXT_FILTER(stripColumnRef); return COLUMN_REF; }
{identifier_quoted} { TEXT_FILTER(stripColumnRef); return QUOTED_COLUMN_REF; }
{white} /* skip blanks and tabs */
Expand Down
13 changes: 5 additions & 8 deletions src/core/qgsexpressionparser.yy
Expand Up @@ -125,7 +125,7 @@ void addParserLocation(YYLTYPE* yyloc, QgsExpressionNode *node)
// tokens for conditional expressions
%token CASE WHEN THEN ELSE END

%token <text> STRING COLUMN_REF FUNCTION SPECIAL_COL VARIABLE NAMED_NODE
%token <text> STRING QUOTED_COLUMN_REF NAME SPECIAL_COL VARIABLE NAMED_NODE

%token COMMA

Expand Down Expand Up @@ -203,14 +203,12 @@ expression:
| expression CONCAT expression { $$ = BINOP($2, $1, $3); }
| NOT expression { $$ = new QgsExpressionNodeUnaryOperator($1, $2); }
| '(' expression ')' { $$ = $2; }
| FUNCTION '(' exp_list ')'
| NAME '(' exp_list ')'
{
int fnIndex = QgsExpression::functionIndex(*$1);
delete $1;
if (fnIndex == -1)
{
// this should not actually happen because already in lexer we check whether an identifier is a known function
// (if the name is not known the token is parsed as a column)
QgsExpression::ParserError::ParserErrorType errorType = QgsExpression::ParserError::FunctionUnknown;
parser_ctx->currentErrorType = errorType;
exp_error(&yyloc, parser_ctx, "Function is not known");
Expand Down Expand Up @@ -240,14 +238,12 @@ expression:
addParserLocation(&@1, $$);
}

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

// columns
| COLUMN_REF { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }
| NAME { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }
| QUOTED_COLUMN_REF { $$ = new QgsExpressionNodeColumnRef( *$1 ); delete $1; }

// special columns (actually functions with no arguments)
| SPECIAL_COL
Expand Down
22 changes: 20 additions & 2 deletions tests/src/core/testqgsexpression.cpp
Expand Up @@ -206,7 +206,7 @@ class TestQgsExpression: public QObject
QTest::newRow( "invalid character" ) << "@" << false;
QTest::newRow( "invalid column reference" ) << "my col" << false;
QTest::newRow( "invalid binary operator" ) << "1+" << false;
QTest::newRow( "invalid function no params" ) << "cos" << false;
QTest::newRow( "invalid function not known no args" ) << "watwat()" << false;
QTest::newRow( "invalid function not known" ) << "coz(1)" << false;
QTest::newRow( "invalid operator IN" ) << "n in p" << false;
QTest::newRow( "empty node list" ) << "1 in ()" << false;
Expand Down Expand Up @@ -1362,10 +1362,12 @@ class TestQgsExpression: public QObject
fields.append( QgsField( QStringLiteral( "x1" ) ) );
fields.append( QgsField( QStringLiteral( "x2" ) ) );
fields.append( QgsField( QStringLiteral( "foo" ), QVariant::Int ) );
fields.append( QgsField( QStringLiteral( "sin" ), QVariant::Int ) );

QgsFeature f;
f.initAttributes( 3 );
f.initAttributes( 4 );
f.setAttribute( 2, QVariant( 20 ) );
f.setAttribute( 3, QVariant( 10 ) );

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

Expand All @@ -1385,6 +1387,22 @@ class TestQgsExpression: public QObject
QCOMPARE( exp2.hasEvalError(), true );
QVariant res2 = exp2.evaluate( &context );
QCOMPARE( res2.type(), QVariant::Invalid );

// Has field called sin and function
QgsExpression exp3( QStringLiteral( "sin" ) );
prepareRes = exp3.prepare( &context );
QCOMPARE( prepareRes, true );
QCOMPARE( exp3.hasEvalError(), false );
res = exp3.evaluate( &context );
QCOMPARE( res.type(), QVariant::Int );
QCOMPARE( res.toInt(), 10 );

QgsExpression exp4( QStringLiteral( "sin(3.14)" ) );
prepareRes = exp4.prepare( &context );
QCOMPARE( prepareRes, true );
QCOMPARE( exp4.hasEvalError(), false );
res = exp4.evaluate( &context );
QCOMPARE( res.toInt(), 0 );
}

void eval_feature_id()
Expand Down

0 comments on commit 409ec85

Please sign in to comment.