You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/09/10 23:10:48 UTC
[GitHub] [phoenix] gokceni opened a new pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
gokceni opened a new pull request #875:
URL: https://github.com/apache/phoenix/pull/875
…heme
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #875:
URL: https://github.com/apache/phoenix/pull/875#discussion_r486776349
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #875:
URL: https://github.com/apache/phoenix/pull/875#discussion_r486776349
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gokceni closed pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
gokceni closed pull request #875:
URL: https://github.com/apache/phoenix/pull/875
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on pull request #875:
URL: https://github.com/apache/phoenix/pull/875#issuecomment-730854135
@gokceni , @kadirozde , I'm curious what the status of PHOENIX-6120 is? Looks like it was approved but never merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #875:
URL: https://github.com/apache/phoenix/pull/875#discussion_r486776349
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #875:
URL: https://github.com/apache/phoenix/pull/875#discussion_r486776349
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -1030,35 +1032,40 @@ public Put buildUpdateMutation(KeyValueBuilder kvBuilder, ValueGetter valueGette
for (Pair<ColumnReference, ColumnReference> colRefPair : colRefPairs) {
ColumnReference indexColRef = colRefPair.getFirst();
ColumnReference dataColRef = colRefPair.getSecond();
- Expression expression = new SingleCellColumnExpression(new PDatum() {
- @Override
- public boolean isNullable() {
- return false;
- }
-
- @Override
- public SortOrder getSortOrder() {
- return null;
- }
-
- @Override
- public Integer getScale() {
- return null;
- }
-
- @Override
- public Integer getMaxLength() {
- return null;
- }
-
- @Override
- public PDataType getDataType() {
- return null;
+
+ byte[] value = null;
+ if (this.dataTableImmutableStorageScheme == this.immutableStorageScheme) {
Review comment:
It is not clear to me how this extra check is useful here and how it fixes the issue. Why is this check necessary, given that the index is supposed to inherit the storage scheme from the data table currently? Is it possible that they can be different.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gokceni commented on pull request #875: PHOENIX-6120 Covered column handling for SINGLE_CELL_ARRAY storage sc…
Posted by GitBox <gi...@apache.org>.
gokceni commented on pull request #875:
URL: https://github.com/apache/phoenix/pull/875#issuecomment-731295993
@gjacoby126 I will close this PR and open another one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org