[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
Github user asfgit closed the pull request at: https://github.com/apache/carbondata/pull/2826 ---
[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2826#discussion_r228542847
--- Diff:
integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CarbonScalaUtil.scala
---
@@ -671,4 +671,20 @@ object CarbonScalaUtil {
dataType == StringType
}
+ /**
+ * Rearrange the column schema with all the sort columns at first. In
case of ALTER ADD COLUMNS,
+ * if the newly added column is a sort column it will be at the last.
But we expects all the
+ * SORT_COLUMNS always at first
+ *
+ * @param columnSchemas
+ * @return
+ */
+ def reArrangeColumnSchema(columnSchemas: mutable.Buffer[ColumnSchema]):
mutable
+ .Buffer[ColumnSchema] = {
--- End diff --
alterTableModel.tableProperties.get("sort_columns") will give size sort
columns,
so based on this we can traverse only once and set both sort and non-sort
columns in buffer, just set sort columns in beginning and non-sort column index
start from size of sortcolumn.
---
[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
Github user ajantha-bhat commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2826#discussion_r228500224
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RangeValueFilterExecuterImpl.java
---
@@ -511,7 +511,7 @@ private BitSet setFilterdIndexToBitSetWithColumnIndex(
// Binary Search cannot be done on '@NU#LL$!", so need to check and
compare for null on
// matching row.
-if (dimensionColumnPage.isNoDicitionaryColumn()) {
+if (dimensionColumnPage.isNoDicitionaryColumn() &&
!dimensionColumnPage.isAdaptiveEncoded()) {
--- End diff --
@jackylk : yes, we can have a BitSet in columnPage for enum of Encoding,
and from PageMetaData encodings we can set the bitset. And to check whether
particular encoding is there or not, we just have to check whether that bit is
set or not.
Currently we don't need this, but like you said, it will help for future
changes. I can do this change if you and @ravipesala agrees for this change.
---
[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
Github user dhatchayani commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2826#discussion_r228498210
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RangeValueFilterExecuterImpl.java
---
@@ -511,7 +511,7 @@ private BitSet setFilterdIndexToBitSetWithColumnIndex(
// Binary Search cannot be done on '@NU#LL$!", so need to check and
compare for null on
// matching row.
-if (dimensionColumnPage.isNoDicitionaryColumn()) {
+if (dimensionColumnPage.isNoDicitionaryColumn() &&
!dimensionColumnPage.isAdaptiveEncoded()) {
--- End diff --
@jackylk isAdaptiveEncoded() is already set/decided by the function
org.apache.carbondata.core.datastore.chunk.reader.dimension.v3.CompressedDimensionChunkFileBasedReaderV3#isEncodedWithAdaptiveMeta.
In future also we can change the function accordingly
---
[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2826#discussion_r228056940
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RangeValueFilterExecuterImpl.java
---
@@ -511,7 +511,7 @@ private BitSet setFilterdIndexToBitSetWithColumnIndex(
// Binary Search cannot be done on '@NU#LL$!", so need to check and
compare for null on
// matching row.
-if (dimensionColumnPage.isNoDicitionaryColumn()) {
+if (dimensionColumnPage.isNoDicitionaryColumn() &&
!dimensionColumnPage.isAdaptiveEncoded()) {
--- End diff --
Not related to this PR. But I think we better have a function to return the
encoding type of the columnPage instead of having `isAdaptiveEncoded`, since we
will add more encoding in the future.
@ravipesala @ajantha-bhat please check
---
[GitHub] carbondata pull request #2826: [CARBONDATA-3023] Alter add column issue with...
GitHub user dhatchayani opened a pull request: https://github.com/apache/carbondata/pull/2826 [CARBONDATA-3023] Alter add column issue with reading a row Be sure to do all of the following checklist to help us incorporate your contribution quickly and easily: - [ ] Any interfaces changed? - [ ] Any backward compatibility impacted? - [ ] Document update required? - [x] Testing done UT Added - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dhatchayani/carbondata CARBONDATA-3023 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/carbondata/pull/2826.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2826 commit 8269397a19d8d98b777f4cc8715354f1dbef9b55 Author: dhatchayani Date: 2018-10-17T10:52:39Z [CARBONDATA-3023] Alter add column issue with reading a row ---
