Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Jun 24, 2015

Currently, we use GenericRow both for Row and InternalRow, which is confusing because it could contain Scala type also Catalyst types.

This PR changes to use GenericInternalRow for InternalRow (contains catalyst types), GenericRow for Row (contains Scala types).

Also fixes some incorrect use of InternalRow or Row.

Davies Liu added 2 commits June 24, 2015 16:37
Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala
@SparkQA
Copy link

SparkQA commented Jun 25, 2015

Test build #35740 has finished for PR 7003 at commit defe931.

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

@SparkQA
Copy link

SparkQA commented Jun 25, 2015

Test build #35737 timed out for PR 7003 at commit 288b31f after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Jun 25, 2015

Test build #960 has finished for PR 7003 at commit defe931.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

should we add final to make sure all InternalRow implementations can not define these functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

@SparkQA
Copy link

SparkQA commented Jun 25, 2015

Test build #35758 timed out for PR 7003 at commit 6f99a97 after a configured wait of 175m.

Davies Liu added 2 commits June 27, 2015 14:39
Conflicts:
	sql/core/src/test/scala/org/apache/spark/sql/sources/TableScanSuite.scala
@SparkQA
Copy link

SparkQA commented Jun 27, 2015

Test build #35901 has finished for PR 7003 at commit bd4e99c.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35903 timed out for PR 7003 at commit d2ebd72 after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35908 has finished for PR 7003 at commit 87b13cf.

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

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35916 has finished for PR 7003 at commit efd0b25.

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

@JoshRosen
Copy link
Contributor

Jenkins, retest this please.

@JoshRosen
Copy link
Contributor

(That test failure is my fault; already hotfixed)

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #969 has finished for PR 7003 at commit efd0b25.

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

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35921 has finished for PR 7003 at commit efd0b25.

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

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #970 has finished for PR 7003 at commit efd0b25.

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

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35934 has finished for PR 7003 at commit d05866c.

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

@cloud-fan
Copy link
Contributor

LGTM.

For data source API, I understand we need an efficient way to cast between InternalRow and Row. But I'm still feel we should make Row and InternalRow siblings so that we can ensure that we don't forget to do the row transformation at compile time. I'll look into it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need SparkSqlSerializer to serialize GenericRow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't looked the details, just follow the old code.

@davies
Copy link
Contributor Author

davies commented Jun 28, 2015

In order to reduce the conflicts, I'm merging this into master.

@asfgit asfgit closed this in 77da5be 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