Skip to content

Conversation

@cloud-fan
Copy link
Contributor

we can use abstract class instead of trait for MutableRow, and remove the dummy java row classes. This PR also contains some small cleanup.

I keep primitive type order as "boolean, byte, short, int, long, float, double", is this conventional?

@cloud-fan
Copy link
Contributor Author

cc @rxin @davies

I can take over the following internal/external row splitting job if @davies is too busy recently.

Some thoughts about future plans:

  • create GenericInternalRow and apply it to the right place. Based on [SPARK-8610] [SQL] Separate Row and InternalRow (part 2) #7003
  • make Row and InternalRow siblings instead of parent and child.(may break this into several parts after digging into)
  • remove unnecessary functions from InternalRow.
  • improve the access control of internal row related stuff, so that we can hide them outside spark sql module, and only expose Row.

@davies
Copy link
Contributor

davies commented Jun 27, 2015

@cloud-fan Thanks for looking into this, We have some offline discussion last week, because of the data source API, we have to make InternalRow can be casted into Row for performance, if we don't change the data source API much. So, we still need to make InternalRow as child of Row, so abort the #6869.

I like the idea of removing BaseMutableRow, will merging these into #7003, if you don't mind. Just came back from retreat, will update that PR soon.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jun 27, 2015

Test build #35895 has started for PR 7059 at commit f7b5308.

@SparkQA
Copy link

SparkQA commented Jun 27, 2015

Test build #35895 has finished for PR 7059 at commit f7b5308.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@cloud-fan
Copy link
Contributor Author

closing in favor of #7003

@cloud-fan cloud-fan closed this Jun 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants