Skip to content

[SPARK-54412][SQL][TEST] Clean up v properly in identifier-clause.sql SQL golden file #53121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+43 −0
Closed

[SPARK-54412][SQL][TEST] Clean up v properly in identifier-clause.sql SQL golden file #53121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause-legacy.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,19 @@ org.apache.spark.sql.catalyst.ExtendedAnalysisException
}


-- !query
DROP VIEW v
-- !query analysis
org.apache.spark.sql.catalyst.analysis.NoSuchTableException
{
"errorClass" : "TABLE_OR_VIEW_NOT_FOUND",
"sqlState" : "42P01",
"messageParameters" : {
"relationName" : "`spark_catalog`.`default`.`v`"
}
}


-- !query
CREATE TABLE tab(IDENTIFIER('c1') INT) USING CSV
-- !query analysis
Expand Down
6 changes: 6 additions & 0 deletions sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,12 @@ Project [c1#x]
+- LocalRelation [col1#x]


-- !query
DROP VIEW v
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`v`, false, true, false


-- !query
CREATE TABLE tab(IDENTIFIER('c1') INT) USING CSV
-- !query analysis
Expand Down
1 change: 1 addition & 0 deletions sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ SELECT my_table.* FROM VALUES (1, 2) AS IDENTIFIER('my_table')(IDENTIFIER('c1'),
WITH identifier('v')(identifier('c1')) AS (VALUES(1)) (SELECT c1 FROM v);
CREATE OR REPLACE VIEW v(IDENTIFIER('c1')) AS VALUES(1);
SELECT c1 FROM v;
DROP VIEW v;
Copy link
Contributor

@cloud-fan cloud-fan Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DROP VIEW v;
DROP VIEW IF EXISTS v;

Copy link
Contributor

@srielau srielau Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's weird. Why could this be needed) There is something else hiding here. Unless the test files are executed in parallel. In that case we have a bigger problem.

Copy link
Contributor

@srielau srielau Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh never mind.. The DROP VIEW was missing.

Copy link
Member Author

@vrozov vrozov Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan I don't think that IF EXIST is preferred in this case

  • view is created just 2 lines above
  • if create view errors out, it is expected that select and drop would also error out
  • all drop view in this file do not use IF EXIST

Copy link
Contributor

@cloud-fan cloud-fan Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is to not let DROP VIEW error out. It's just for cleanup the test resource, there is no point to test its error behavior.

Copy link
Member Author

@vrozov vrozov Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not matter here if DROP VIEW v errors out or not. The goal is to cleanup and if the view does not exist (in the legacy case), both SELECT and DROP would error out consistently. The error does not affect anything.

Copy link
Contributor

@cloud-fan cloud-fan Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error gets committed into the golden file...

Copy link
Member Author

@vrozov vrozov Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same for SELECT one line above.

Copy link
Contributor

@cloud-fan cloud-fan Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup code should be as less noisy as possible. Maybe we should have a clear separation between testing code and cleanup code in the golden file test framework, so that we don't commit anything for the cleanup code in the golden file. The SELECT one line above is clearly a testing code.

Thanks @dongjoon-hyun for quickly addressing this issue properly!

CREATE TABLE tab(IDENTIFIER('c1') INT) USING CSV;
INSERT INTO tab(IDENTIFIER('c1')) VALUES(1);
SELECT c1 FROM tab;
Expand Down
15 changes: 15 additions & 0 deletions sql/core/src/test/resources/sql-tests/results/identifier-clause-legacy.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,21 @@ org.apache.spark.sql.catalyst.ExtendedAnalysisException
}


-- !query
DROP VIEW v
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.catalyst.analysis.NoSuchTableException
{
"errorClass" : "TABLE_OR_VIEW_NOT_FOUND",
"sqlState" : "42P01",
"messageParameters" : {
"relationName" : "`spark_catalog`.`default`.`v`"
}
}


-- !query
CREATE TABLE tab(IDENTIFIER('c1') INT) USING CSV
-- !query schema
Expand Down
8 changes: 8 additions & 0 deletions sql/core/src/test/resources/sql-tests/results/identifier-clause.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,14 @@ struct<c1:int>
1


-- !query
DROP VIEW v
-- !query schema
struct<>
-- !query output



-- !query
CREATE TABLE tab(IDENTIFIER('c1') INT) USING CSV
-- !query schema
Expand Down