From f2c2eaf013814a4e9fc61f084b556da6c86b9db2 Mon Sep 17 00:00:00 2001 From: Jeremy Dyer Date: Fri, 14 Jan 2022 13:47:11 -0500 Subject: [PATCH 1/4] Modified show.ftl to conditionally expect FROM in parsing logic --- dask_sql/physical/rel/custom/tables.py | 7 ++++++- planner/src/main/codegen/includes/show.ftl | 8 +++++--- .../src/main/java/com/dask/sql/parser/SqlShowTables.java | 5 ++++- tests/integration/test_show.py | 7 +++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/dask_sql/physical/rel/custom/tables.py b/dask_sql/physical/rel/custom/tables.py index 8ec1a2009..2090282b2 100644 --- a/dask_sql/physical/rel/custom/tables.py +++ b/dask_sql/physical/rel/custom/tables.py @@ -29,7 +29,12 @@ class ShowTablesPlugin(BaseRelPlugin): def convert( self, sql: "org.apache.calcite.sql.SqlNode", context: "dask_sql.Context" ) -> DataContainer: - schema = str(sql.getSchema()).split(".")[-1] + schema = sql.getSchema() + if schema is not None: + schema = str(schema).split(".")[-1] + else: + schema = "root" + if schema not in context.schema: raise AttributeError(f"Schema {schema} is not defined.") diff --git a/planner/src/main/codegen/includes/show.ftl b/planner/src/main/codegen/includes/show.ftl index 1f36b5806..b30dd5cb0 100644 --- a/planner/src/main/codegen/includes/show.ftl +++ b/planner/src/main/codegen/includes/show.ftl @@ -22,11 +22,13 @@ SqlNode SqlShowSchemas() : SqlNode SqlShowTables() : { final Span s; - final SqlIdentifier schema; + SqlIdentifier schema = null; } { - { s = span(); } - schema = SimpleIdentifier() + { s = span(); } + ( + schema = SimpleIdentifier() + )? { return new SqlShowTables(s.end(this), schema); } diff --git a/planner/src/main/java/com/dask/sql/parser/SqlShowTables.java b/planner/src/main/java/com/dask/sql/parser/SqlShowTables.java index 5f7301f45..89c22d771 100644 --- a/planner/src/main/java/com/dask/sql/parser/SqlShowTables.java +++ b/planner/src/main/java/com/dask/sql/parser/SqlShowTables.java @@ -7,6 +7,7 @@ import org.apache.calcite.sql.parser.SqlParserPos; public class SqlShowTables extends SqlDescribeSchema { + public SqlShowTables(SqlParserPos pos, SqlIdentifier schemaName) { super(pos, schemaName); } @@ -15,6 +16,8 @@ public SqlShowTables(SqlParserPos pos, SqlIdentifier schemaName) { public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { writer.keyword("SHOW"); writer.keyword("TABLES"); - this.getSchema().unparse(writer, leftPrec, rightPrec); + if (this.getSchema() != null) { + this.getSchema().unparse(writer, leftPrec, rightPrec); + } } } diff --git a/tests/integration/test_show.py b/tests/integration/test_show.py index 893c91738..c3e843df0 100644 --- a/tests/integration/test_show.py +++ b/tests/integration/test_show.py @@ -95,3 +95,10 @@ def test_wrong_input(c): c.sql(f'SHOW COLUMNS FROM "{c.schema_name}"."table"') with pytest.raises(AttributeError): c.sql('SHOW TABLES FROM "wrong"') + + +def test_show_tables_no_schema(c): + df = pd.DataFrame({"id": [0, 1]}) + c.create_table("test", df) + + actual_df = c.sql("show tables").compute() From 8755e93be8d7ba54dfa37c9e66f7c0268cc8408e Mon Sep 17 00:00:00 2001 From: Jeremy Dyer Date: Fri, 14 Jan 2022 13:51:20 -0500 Subject: [PATCH 2/4] Use DEFAULT_SCHEMA_NAME value from context.py instead of a magic string --- dask_sql/physical/rel/custom/tables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dask_sql/physical/rel/custom/tables.py b/dask_sql/physical/rel/custom/tables.py index 2090282b2..e6cc16477 100644 --- a/dask_sql/physical/rel/custom/tables.py +++ b/dask_sql/physical/rel/custom/tables.py @@ -33,7 +33,7 @@ def convert( if schema is not None: schema = str(schema).split(".")[-1] else: - schema = "root" + schema = context.DEFAULT_SCHEMA_NAME if schema not in context.schema: raise AttributeError(f"Schema {schema} is not defined.") From 38f7a3ab8e5f5341fd7affd7f904b05096fc78ba Mon Sep 17 00:00:00 2001 From: Jeremy Dyer Date: Fri, 14 Jan 2022 13:57:27 -0500 Subject: [PATCH 3/4] add assert making sure test table is present --- tests/integration/test_show.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_show.py b/tests/integration/test_show.py index c3e843df0..1c02aa313 100644 --- a/tests/integration/test_show.py +++ b/tests/integration/test_show.py @@ -102,3 +102,4 @@ def test_show_tables_no_schema(c): c.create_table("test", df) actual_df = c.sql("show tables").compute() + assert len(actual_df[actual_df["Table"] == "test"]) > 0 From 6bcd2f374dd36f7ad3a7592dadfcea043e9bbc66 Mon Sep 17 00:00:00 2001 From: Jeremy Dyer Date: Sat, 15 Jan 2022 10:17:34 -0500 Subject: [PATCH 4/4] changes based on review for test layout --- tests/integration/test_show.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_show.py b/tests/integration/test_show.py index 1c02aa313..6e2af33d1 100644 --- a/tests/integration/test_show.py +++ b/tests/integration/test_show.py @@ -2,6 +2,8 @@ import pytest from pandas.testing import assert_frame_equal +from dask_sql import Context + try: import cudf except ImportError: @@ -98,8 +100,11 @@ def test_wrong_input(c): def test_show_tables_no_schema(c): + c = Context() + df = pd.DataFrame({"id": [0, 1]}) c.create_table("test", df) actual_df = c.sql("show tables").compute() - assert len(actual_df[actual_df["Table"] == "test"]) > 0 + expected_df = pd.DataFrame({"Table": ["test"]}) + assert_frame_equal(actual_df, expected_df)