SQL: Implement IIF(<cond>, <result1>, <result2>) #41420

matriv · 2019-04-22T18:33:12Z

Implement a more trivial case of the CASE expression which is
expressed as a traditional function with 2 or 3 arguments. e.g.:

IIF(a = 1, 'one', 'many')

IIF(a > 0, 'positive')

Closes: #40917

elasticmachine · 2019-04-22T18:33:13Z

Pinging @elastic/es-search

Implement a more trivial case of the `CASE` expression which is expressed as a traditional function with 2 or 3 arguments. e.g.: ``` IF(a = 1, 'one', 'many') ``` ``` IF(a > 0, 'positive) ``` Closes: elastic#40917

costin

LGTM.
The only comment is whether this should be only IIF instead of IF since:
Oracle and SQL Server has a different syntax for IF (IF THEN)
SQL Server does support IIF in this form
Postgres does not support IF

Essentially only MySQL has this flavor of IF and frankly I would go after the IF THEN format in SQLServer/Oracle instead and drop this one since there would be too many choices IMO.

costin · 2019-04-23T05:41:25Z

...lugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/If.java

+
+public class If extends Case {
+
+    public If(Source source, Expression condition, Expression result, Expression defaultValue) {


defaultValue should be really else across the whole hierarchy. (if then else - vs if then default).

costin · 2019-04-23T05:42:22Z

x-pack/plugin/sql/qa/src/main/resources/conditionals.csv-spec

+ifGroupByAndHaving
+schema::count:l|gender:s|languages:byte
+SELECT count(*) AS count, gender, languages FROM test_emp
+GROUP BY 2, 3 HAVING IF(count(*) > 10, 'many', 'a few') = 'many'


matriv · 2019-04-23T07:08:39Z

IF indeed is only a MySQL flavour, but Imho doesn't hurt to have it together with IIF. The IF... THEN... ELSE... END IF statement comes from Oracle PL/SQL (declare functions/procedures, etc.) and not in the standard query language. Also for SQL server: https://docs.microsoft.com/en-us/sql/t-sql/language-elements/else-if-else-transact-sql?view=sql-server-2017 it's for control flow inside code blocks with loops, declared methods and variables, etc.

@astefan what do you think?

astefan

LGTM, but I left few small comments.

Regarding IIF vs. IF debate, I would keep IIF only. On one hand we have a bunch of aliases for all the date parts functions - ie "DAY_OF_MONTH", "DAYOFMONTH", "DAY", "DOM" where DAYOFMONTH is the "true" function name and others being just aliases we added for ease of use. But all these aliases do have the same functional behavior are cannot be confused with something else, whereas IF could potentially have a different meaning based on other SQL providers. And if we keep IF, some users might want to use it as they know it's used in Oracle PL/SQL.

astefan · 2019-04-23T07:22:20Z

docs/reference/sql/functions/conditional.asciidoc

+
+[TIP]
+=================
+If functions can be combined to implement more complex logic simulating the <<sql-functions-conditional-case>>


Capital letters IF.

astefan · 2019-04-23T07:33:24Z

x-pack/plugin/sql/qa/src/main/resources/docs/docs.csv-spec

@@ -2095,6 +2097,51 @@ elastic
 ;




I would like to see, based on a previous issue fixed in the past, a query where you use a CONVERT without the optional parameter for IIF, with an alias and grouped by alias: SELECT CONVERT(IIF(languages > 1, IIF(languages = 3, '3')), SQL_BIGINT) AS cond FROM test_emp GROUP BY cond ORDER BY cond DESC.

astefan · 2019-04-23T07:36:34Z

.../sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/IfTests.java

+
+    private List<Expression> mutateChildren(If iif) {
+        List<Expression> expressions = new ArrayList<>(3);
+        expressions.add(new Equals(randomSource(), randomStringLiteral(), randomStringLiteral()));


The Equals here shouldn't have either first string different from the original one, either the second one, either both? I know there are small chances for this to happen in tests, but it can happen.

It can be but the chances are really small and on top we are changing for sure the thenResult and the elseResult. But anyway for 100% correctness I'm changing it to create also a different Equals condition.

Implement a more trivial case of the CASE expression which is expressed as a traditional function with 2 or 3 arguments. e.g.: IIF(a = 1, 'one', 'many') IIF(a > 0, 'positive') Closes: #40917 (cherry picked from commit add02f4)

Implement a more trivial case of the CASE expression which is expressed as a traditional function with 2 or 3 arguments. e.g.: IIF(a = 1, 'one', 'many') IIF(a > 0, 'positive') Closes: elastic#40917

matriv added >enhancement :Analytics/SQL SQL querying v8.0.0 v7.2.0 labels Apr 22, 2019

matriv requested review from costin and astefan April 22, 2019 18:33

SQL: Implement IF(<cond>, <result1>, <result2>)

46989c1

Implement a more trivial case of the `CASE` expression which is expressed as a traditional function with 2 or 3 arguments. e.g.: ``` IF(a = 1, 'one', 'many') ``` ``` IF(a > 0, 'positive) ``` Closes: elastic#40917

matriv force-pushed the impl-if branch from 2fbbf12 to 46989c1 Compare April 22, 2019 19:26

costin approved these changes Apr 23, 2019

View reviewed changes

matriv and others added 2 commits April 23, 2019 10:20

Rename variables

aefc0fe

Merge remote-tracking branch 'upstream/master' into impl-if

e0016f0

astefan approved these changes Apr 23, 2019

View reviewed changes

matriv changed the title ~~SQL: Implement IF(<cond>, <result1>, <result2>)~~ SQL: Implement IIF(<cond>, <result1>, <result2>) Apr 23, 2019

matriv added 4 commits April 23, 2019 12:08

Keep only IIF, remove IF

24400e3

fix test results

006c709

Fix test result

c761abc

remove empty line

2e8e72d

matriv merged commit add02f4 into elastic:master Apr 23, 2019

matriv deleted the impl-if branch April 23, 2019 13:30

jrodewig mentioned this pull request Apr 23, 2019

[DOCS] Remove inline callouts for Asciidoctor migration #41460

Merged

matriv mentioned this pull request Jun 7, 2019

Add if support for sql query #42959

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SQL: Implement IIF(<cond>, <result1>, <result2>) #41420

SQL: Implement IIF(<cond>, <result1>, <result2>) #41420

Uh oh!

matriv commented Apr 22, 2019 •

edited

Loading

Uh oh!

elasticmachine commented Apr 22, 2019

Uh oh!

costin left a comment

Uh oh!

costin Apr 23, 2019

Uh oh!

costin Apr 23, 2019

Uh oh!

matriv commented Apr 23, 2019

Uh oh!

astefan left a comment

Uh oh!

astefan Apr 23, 2019

Uh oh!

astefan Apr 23, 2019 •

edited

Loading

Uh oh!

astefan Apr 23, 2019

Uh oh!

matriv Apr 23, 2019

Uh oh!

Uh oh!


		public class If extends Case {

		public If(Source source, Expression condition, Expression result, Expression defaultValue) {

SQL: Implement IIF(<cond>, <result1>, <result2>) #41420

SQL: Implement IIF(<cond>, <result1>, <result2>) #41420

Uh oh!

Conversation

matriv commented Apr 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Apr 22, 2019

Uh oh!

costin left a comment

Choose a reason for hiding this comment

Uh oh!

costin Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

costin Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

matriv commented Apr 23, 2019

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

astefan Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

astefan Apr 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astefan Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

matriv Apr 23, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matriv commented Apr 22, 2019 •

edited

Loading

astefan Apr 23, 2019 •

edited

Loading