Skip to content

Conversation

@Hisoka-X
Copy link
Member

@Hisoka-X Hisoka-X commented May 5, 2023

What changes were proposed in this pull request?

Remove useless code in class ScriptInputOutputSchema. Include getRowFormatSQL, inputRowFormatSQL, outputRowFormatSQL method. It unused when #16869

Why are the changes needed?

Clear code.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unnecessary.

@github-actions github-actions bot added the SQL label May 5, 2023
@HyukjinKwon
Copy link
Member

cc @AngersZhuuuu

@AngersZhuuuu
Copy link
Contributor

First, pls write a jira and assign this pr to jira ticket
These code is used to format sql from LogicalPlan, seems copied from Hive, ok to remove.

@Hisoka-X Hisoka-X changed the title [MINOR][SQL] Remove useless getRowFormatSQL method [SPARK-43405][SQL] Remove useless getRowFormatSQL method May 8, 2023
@Hisoka-X
Copy link
Member Author

Hisoka-X commented May 8, 2023

First, pls write a jira and assign this pr to jira ticket These code is used to format sql from LogicalPlan, seems copied from Hive, ok to remove.

Ok, I created SPARK-43405

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

This PR actually removes inputRowFormatSQL and outputRowFormatSQL method additionally.

  1. Could you revise the PR title and description, @Hisoka-X ?
  2. Do you happen to know when these methods become useless?

@Hisoka-X Hisoka-X changed the title [SPARK-43405][SQL] Remove useless getRowFormatSQL method [SPARK-43405][SQL] Remove useless getRowFormatSQL, inputRowFormatSQL, outputRowFormatSQL method May 9, 2023
@Hisoka-X Hisoka-X changed the title [SPARK-43405][SQL] Remove useless getRowFormatSQL, inputRowFormatSQL, outputRowFormatSQL method [SPARK-43405][SQL] Remove useless getRowFormatSQL, inputRowFormatSQL, outputRowFormatSQL method May 9, 2023
@Hisoka-X
Copy link
Member Author

Hisoka-X commented May 9, 2023

  1. Could you revise the PR title and description, @Hisoka-X ?

Done

  1. Do you happen to know when these methods become useless?

It unused when #16869 .

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented May 10, 2023

Thank you for the info, @Hisoka-X .

cc @jiangxb1987 and @hvanhovell from #16869

@hvanhovell
Copy link
Contributor

@dongjoon-hyun that is looonnnnggg time ago :)

Let me take a look.

Copy link
Contributor

@hvanhovell hvanhovell left a comment

Choose a reason for hiding this comment

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

LGTM

@hvanhovell
Copy link
Contributor

Merging.

@dongjoon-hyun
Copy link
Member

Thank you, @Hisoka-X and @hvanhovell . :)

@Hisoka-X Hisoka-X deleted the remove_useless_code branch May 11, 2023 08:44
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.

5 participants