Skip to content

[MINOR][SQL] Clean up ObjectProducerExec operators - #25065

Closed
jaceklaskowski wants to merge 1 commit into
apache:masterfrom
jaceklaskowski:ObjectProducerExec-operators-cleanup
Closed

[MINOR][SQL] Clean up ObjectProducerExec operators#25065
jaceklaskowski wants to merge 1 commit into
apache:masterfrom
jaceklaskowski:ObjectProducerExec-operators-cleanup

Conversation

@jaceklaskowski

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Cleaned up (removed) code duplication in ObjectProducerExec operators so they use the trait's methods.

How was this patch tested?

Local build. Waiting for Jenkins.

@SparkQA

Copy link
Copy Markdown

Test build #107306 has finished for PR 25065 at commit 52b03b1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun

dongjoon-hyun commented Jul 7, 2019

Copy link
Copy Markdown
Member

Hi, @jaceklaskowski . Could you make a JIRA issue for this? Thanks.

@srowen

Copy link
Copy Markdown
Member

A JIRA won't hurt but this looks like just a code cleanup; there should be no functional change.

@jaceklaskowski

Copy link
Copy Markdown
ContributorAuthor

@dongjoon-hyun When I created the PR I thought to make a JIRA issue, but it's just a simple refactoring (code cleanup) so I don't think it begs for one.

@dongjoon-hyun

Copy link
Copy Markdown
Member

Okay. Then, I leave this PR to the other reviews.

@HyukjinKwon

HyukjinKwon commented Jul 11, 2019

Copy link
Copy Markdown
Member

I'm fine either way. I also prefer to have a JIRA in general for trackability, for instance, backporting stuff. I think this PR is boundary between no JIRA and JIRA :-).

As an example, there was a recent and rather similar change at #25052 with JIRA so it's not a bad idea to have a JIRA.

@srowen

Copy link
Copy Markdown
Member

Merged to master. I think it's fine.

@jaceklaskowski
jaceklaskowski deleted the ObjectProducerExec-operators-cleanup branch July 15, 2019 08:35
vinodkc pushed a commit to vinodkc/spark that referenced this pull request Jul 18, 2019
## What changes were proposed in this pull request?
Cleaned up (removed) code duplication in `ObjectProducerExec` operators so they use the trait's methods.
## How was this patch tested?
Local build. Waiting for Jenkins.
Closesapache#25065 from jaceklaskowski/ObjectProducerExec-operators-cleanup.
Authored-by: Jacek Laskowski <jacek@japila.pl>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for freeto 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

@jaceklaskowski@SparkQA@dongjoon-hyun@srowen@HyukjinKwon