Skip to content

Conversation

@yhuai
Copy link
Contributor

@yhuai yhuai commented Apr 13, 2015

@yhuai
Copy link
Contributor Author

yhuai commented Apr 13, 2015

@liancheng Where should I add the test? At first, I thought NullableColumnBuilderSuite is the place. But, why NullableColumnBuilderSuite does not really use those real column builders?

@SparkQA
Copy link

SparkQA commented Apr 13, 2015

Test build #30199 has finished for PR 5499 at commit d169d33.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@yhuai
Copy link
Contributor Author

yhuai commented Apr 15, 2015

I will add a test in InMemoryColumnarQuerySuite.

@liancheng
Copy link
Contributor

Good catch! The change LGTM.

NullableColumnBuilderSuite uses a mock builder because the real builders also mix in CompressibleColumnBuilder, while NullableColumnBuilderSuite is used to test nulls.

This wasn't detected before because we were using a column type ID in the columnar buffer, and column accessors are constructed according to the type ID. However, while adding column type for fixed-decimal, decimals with different precisions and scales are actually different types, a single type ID is not enough to represent complete type information. Thus column type ID is invalidated and now column accessors are built according to the data types in the schema.

@SparkQA
Copy link

SparkQA commented Apr 15, 2015

Test build #30357 has finished for PR 5499 at commit 84cba38.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@marmbrus
Copy link
Contributor

Thanks! Merged to master.

@asfgit asfgit closed this in 785f955 Apr 15, 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