Skip to content

Conversation

@ajantha-bhat
Copy link
Member

This PR is a backport of #3757 to spark-3.0

@github-actions github-actions bot added the spark label Dec 23, 2021
// Add a dummy prefix linking to the table to collect the resolved spark expression from optimized plan.
val prefix = String.format("SELECT 42 from %s where ", tableName)
val logicalPlan = session.sessionState.sqlParser.parsePlan(prefix + where)
val optimizedLogicalPlan = session.sessionState.executePlan(logicalPlan).optimizedPlan
Copy link
Member Author

Choose a reason for hiding this comment

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

The only change compared to spark-3.2 PR is this. executePlan accepts only one argument in spark-3.0 and spark-3.1


List<Object[]> output = sql(
"CALL %s.system.rewrite_data_files(table => '%s')", catalogName, tableIdent);
"CALL %s.system.rewrite_data_files(table => '%s')", catalogName, tableIdent);
Copy link
Contributor

@kbendick kbendick Dec 24, 2021

Choose a reason for hiding this comment

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

Nit: Usually we try to avoid formatting only changes (even if they are the correct formatting), as the larger diff makes it harder for people that maintain forks to cherry-pick commits. If you aren’t changing the line, I’d suggest leaving it as is.

Same comment applies to all formatting only changes within a PR. 😊

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. But I got a specific comment to fix indentation for this file in the spark-3.2 PR. Hence adopting it here also.

But I think checkstyle should catch over indented code in PR, so it can be caught and fixed in the respective PR itself. Let me explore why it is not catching

@rdblue rdblue merged commit b71ab6f into apache:master Dec 30, 2021
@rdblue
Copy link
Contributor

rdblue commented Dec 30, 2021

Thanks, @ajantha-bhat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants