From 6e693b76a3c678bc400427aa10d055d279ac1137 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:45:55 -0500 Subject: [PATCH] fix(sql): remove constants in `order_by` calls during select merging (#10475) Remove literals in `ORDER BY` when sorting tables. Fixes #10428. --- ibis/backends/sql/rewrites.py | 4 +++- .../bigquery/out.sql | 7 +++++++ .../clickhouse/out.sql | 7 +++++++ .../databricks/out.sql | 7 +++++++ .../datafusion/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/druid/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/duckdb/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/exasol/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/flink/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/impala/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/mssql/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/mysql/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/oracle/out.sql | 7 +++++++ .../postgres/out.sql | 7 +++++++ .../pyspark/out.sql | 7 +++++++ .../risingwave/out.sql | 7 +++++++ .../snowflake/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/sqlite/out.sql | 7 +++++++ .../test_order_by_no_deference_literals/trino/out.sql | 7 +++++++ ibis/backends/tests/test_sql.py | 10 ++++++++++ 20 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/bigquery/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/clickhouse/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/databricks/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/datafusion/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/druid/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/duckdb/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/exasol/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/flink/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/impala/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mssql/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mysql/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/oracle/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/postgres/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/pyspark/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/risingwave/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/snowflake/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/sqlite/out.sql create mode 100644 ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/trino/out.sql diff --git a/ibis/backends/sql/rewrites.py b/ibis/backends/sql/rewrites.py index 6dcdeafc97e0..0526347e456b 100644 --- a/ibis/backends/sql/rewrites.py +++ b/ibis/backends/sql/rewrites.py @@ -320,7 +320,9 @@ def merge_select_select(_, **kwargs): selections=selections, predicates=unique_predicates, qualified=unique_qualified, - sort_keys=unique_sort_keys, + sort_keys=tuple( + key for key in unique_sort_keys if not isinstance(key.expr, ops.Literal) + ), distinct=distinct, ) return result if complexity(result) <= complexity(_) else _ diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/bigquery/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/bigquery/out.sql new file mode 100644 index 000000000000..dedc0ae8059f --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/bigquery/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + `t0`.`a` ASC NULLS LAST \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/clickhouse/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/clickhouse/out.sql new file mode 100644 index 000000000000..f83550410464 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/clickhouse/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a" AS "a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/databricks/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/databricks/out.sql new file mode 100644 index 000000000000..dedc0ae8059f --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/databricks/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + `t0`.`a` ASC NULLS LAST \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/datafusion/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/datafusion/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/datafusion/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/druid/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/druid/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/druid/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/duckdb/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/duckdb/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/duckdb/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/exasol/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/exasol/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/exasol/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/flink/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/flink/out.sql new file mode 100644 index 000000000000..dedc0ae8059f --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/flink/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + `t0`.`a` ASC NULLS LAST \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/impala/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/impala/out.sql new file mode 100644 index 000000000000..56acf13d5c8d --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/impala/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + `t0`.`a` ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mssql/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mssql/out.sql new file mode 100644 index 000000000000..0474550f8089 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mssql/out.sql @@ -0,0 +1,7 @@ +SELECT + [t0].[a], + 9 AS [i], + 'foo' AS [s] +FROM [test] AS [t0] +ORDER BY + CASE WHEN [t0].[a] IS NULL THEN 1 ELSE 0 END, [t0].[a] ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mysql/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mysql/out.sql new file mode 100644 index 000000000000..dea7ffd250fc --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/mysql/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + CASE WHEN `t0`.`a` IS NULL THEN 1 ELSE 0 END, `t0`.`a` ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/oracle/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/oracle/out.sql new file mode 100644 index 000000000000..1d506b567236 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/oracle/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/postgres/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/postgres/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/postgres/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/pyspark/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/pyspark/out.sql new file mode 100644 index 000000000000..dedc0ae8059f --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/pyspark/out.sql @@ -0,0 +1,7 @@ +SELECT + `t0`.`a`, + 9 AS `i`, + 'foo' AS `s` +FROM `test` AS `t0` +ORDER BY + `t0`.`a` ASC NULLS LAST \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/risingwave/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/risingwave/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/risingwave/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/snowflake/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/snowflake/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/snowflake/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/sqlite/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/sqlite/out.sql new file mode 100644 index 000000000000..e0cfd7cd3fed --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/sqlite/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC NULLS LAST \ No newline at end of file diff --git a/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/trino/out.sql b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/trino/out.sql new file mode 100644 index 000000000000..20d130d68814 --- /dev/null +++ b/ibis/backends/tests/snapshots/test_sql/test_order_by_no_deference_literals/trino/out.sql @@ -0,0 +1,7 @@ +SELECT + "t0"."a", + 9 AS "i", + 'foo' AS "s" +FROM "test" AS "t0" +ORDER BY + "t0"."a" ASC \ No newline at end of file diff --git a/ibis/backends/tests/test_sql.py b/ibis/backends/tests/test_sql.py index 748addce3014..bcb09dd11012 100644 --- a/ibis/backends/tests/test_sql.py +++ b/ibis/backends/tests/test_sql.py @@ -259,3 +259,13 @@ def test_sample(backend_name, snapshot, subquery): row = ibis.to_sql(t.sample(0.5, method="row"), dialect=backend_name) snapshot.assert_match(block, "block.sql") snapshot.assert_match(row, "row.sql") + + +@pytest.mark.parametrize("backend_name", _get_backends_to_test()) +@pytest.mark.notimpl(["polars"], raises=ValueError, reason="not a SQL backend") +def test_order_by_no_deference_literals(backend_name, snapshot): + t = ibis.table({"a": "int"}, name="test") + s = t.select("a", i=ibis.literal(9), s=ibis.literal("foo")) + o = s.order_by("a", "i", "s") + sql = ibis.to_sql(o, dialect=backend_name) + snapshot.assert_match(sql, "out.sql")