Skip to content

[SQL] Construct the MutableRow from an Array #3217

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[SQL] Construct the MutableRow from an Array #3217

wants to merge 3 commits into from

Conversation

chenghao-intel
Copy link
Contributor

No description provided.

@SparkQA
Copy link

SparkQA commented Nov 12, 2014

Test build #23238 has started for PR 3217 at commit a751838.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 12, 2014

Test build #23240 has started for PR 3217 at commit 4681aea.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 12, 2014

Test build #23238 has finished for PR 3217 at commit a751838.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class GenericMutableRow(v: Array[Any]) extends GenericRow(v) with MutableRow

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23238/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Nov 12, 2014

Test build #23240 has finished for PR 3217 at commit 4681aea.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class GenericMutableRow(v: Array[Any]) extends GenericRow(v) with MutableRow

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23240/
Test PASSed.

marmbrus
marmbrus reviewed Nov 14, 2014
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Row.scala
/**
* This method can be used to construct a [[MutableRow]] from a [[Row]]
*/
def toMutableRow(row: Row): MutableRow = row match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this private[sql]? I don't think we want to expose mutable rows to users yet as the API is still in flux.

Copy link
Contributor

Choose a reason for hiding this comment

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

And actually, why is this here as a static method instead of an instance method on Row itself? What is your intended use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exposing the mutable row is required in #3247, we want to reuse the row object as aggregation buffer, but you're right, we should make it as instance method on Row itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, probably I can use the SpecificMutableRow, I will simplify the PR by fixing the GenericMutableRow only.

@chenghao-intel chenghao-intel changed the title [SQL] Construct the MutableRow from an existed row [SQL] Construct the MutableRow from an Array Nov 17, 2014
@chenghao-intel
Copy link
Contributor Author

Thank you @marmbrus , I've updated the title, and revert the change of the Row object, I will try to use the SpecificMutableRow instead in another case, let's see if there any more issue with that then.

@SparkQA
Copy link

SparkQA commented Nov 17, 2014

Test build #23447 has started for PR 3217 at commit e8a10bd.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 17, 2014

Test build #23447 has finished for PR 3217 at commit e8a10bd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class GenericMutableRow(v: Array[Any]) extends GenericRow(v) with MutableRow

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23447/
Test PASSed.

@marmbrus
Copy link
Contributor

Thanks! Merged to master and 1.2

@asfgit asfgit closed this in 69e858c Nov 18, 2014
asfgit pushed a commit that referenced this pull request Nov 18, 2014
@chenghao-intel @marmbrus
[SQL] Construct the MutableRow from an Array
7d04426 
Author: Cheng Hao <[email protected]>

Closes #3217 from chenghao-intel/mutablerow and squashes the following commits:

e8a10bd [Cheng Hao] revert the change of Row object
4681aea [Cheng Hao] Add toMutableRow method in object Row
a751838 [Cheng Hao] Construct the MutableRow from an existed row

(cherry picked from commit 69e858c)
Signed-off-by: Michael Armbrust <[email protected]>
@chenghao-intel chenghao-intel deleted the mutablerow branch July 2, 2015 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chenghao-intel @SparkQA @AmplabJenkins @marmbrus