[
https://issues.apache.org/jira/browse/PHOENIX-6365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17410207#comment-17410207
]
ASF GitHub Bot commented on PHOENIX-6365:
-----------------------------------------
virajjasani commented on a change in pull request #1133:
URL: https://github.com/apache/phoenix/pull/1133#discussion_r702445575
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1121,46 +1124,55 @@ public PSchema resolveSchema(String schemaName) throws
SQLException {
return null;
}
}
-
+
private static class ProjectedTableColumnResolver extends
MultiTableColumnResolver {
private final boolean isLocalIndex;
+ // We must handle the local index data tables separately
+ protected final ListMultimap<String, TableRef> localIndexDataTableMap
= ArrayListMultimap.<String, TableRef> create();
+ protected final List<TableRef> localIndexDataTables = new
ArrayList<>();;
Review comment:
nit: `;;`
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
##########
@@ -298,6 +306,7 @@ private static void
projectIndexColumnFamily(StatementContext context, String cf
PColumn indexColumn = null;
ColumnRef ref = null;
String indexColumnFamily = null;
+ boolean localIndexResolved = true;
Review comment:
same as above?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1121,46 +1124,55 @@ public PSchema resolveSchema(String schemaName) throws
SQLException {
return null;
}
}
-
+
private static class ProjectedTableColumnResolver extends
MultiTableColumnResolver {
private final boolean isLocalIndex;
+ // We must handle the local index data tables separately
+ protected final ListMultimap<String, TableRef> localIndexDataTableMap
= ArrayListMultimap.<String, TableRef> create();
+ protected final List<TableRef> localIndexDataTables = new
ArrayList<>();;
private final List<TableRef> theTableRefs;
private final Map<ColumnRef, Integer> columnRefMap;
private ProjectedTableColumnResolver(PTable projectedTable,
PhoenixConnection conn, Map<String, UDFParseNode> udfParseNodes) throws
SQLException {
+
super(conn, 0, udfParseNodes, null);
Preconditions.checkArgument(projectedTable.getType() ==
PTableType.PROJECTED);
this.isLocalIndex = projectedTable.getIndexType() ==
IndexType.LOCAL;
this.columnRefMap = new HashMap<ColumnRef, Integer>();
long ts = Long.MAX_VALUE;
for (int i = projectedTable.getBucketNum() == null ? 0 : 1; i <
projectedTable.getColumns().size(); i++) {
PColumn column = projectedTable.getColumns().get(i);
- ColumnRef colRef = ((ProjectedColumn)
column).getSourceColumnRef();
- TableRef tableRef = colRef.getTableRef();
- if (!tables.contains(tableRef)) {
- String alias = tableRef.getTableAlias();
- if (alias != null) {
- this.tableMap.put(alias, tableRef);
- }
- String name = tableRef.getTable().getName().getString();
- if (alias == null || !alias.equals(name)) {
- tableMap.put(name, tableRef);
- }
- tables.add(tableRef);
- if (tableRef.getLowerBoundTimeStamp() < ts) {
- ts = tableRef.getLowerBoundTimeStamp();
- }
+ ColumnRef sourceColRef = ((ProjectedColumn)
column).getSourceColumnRef();
+ TableRef tableRef = sourceColRef.getTableRef();
+
+ if (sourceColRef instanceof LocalIndexDataColumnRef &&
!localIndexDataTables.contains(tableRef)) {
+ rememberLocalIndexDataTable(tableRef,
tableRef.getTableAlias());
+ } else
+ if (!tables.contains(tableRef)) {
+ rememberTable(tableRef, tableRef.getTableAlias());
+ }
Review comment:
I got confused first time, I think we can club else and if here?
```
} else if (!tables.contains(tableRef)) {
rememberTable(tableRef, tableRef.getTableAlias());
}
```
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1121,46 +1124,55 @@ public PSchema resolveSchema(String schemaName) throws
SQLException {
return null;
}
}
-
+
private static class ProjectedTableColumnResolver extends
MultiTableColumnResolver {
private final boolean isLocalIndex;
+ // We must handle the local index data tables separately
+ protected final ListMultimap<String, TableRef> localIndexDataTableMap
= ArrayListMultimap.<String, TableRef> create();
Review comment:
nit: `ArrayListMultimap.create()` would do?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -985,7 +976,17 @@ public TableRef refreshDerivedTableNode(DerivedTableNode
derivedTableNode) throw
return this.resolveTable(null, tableAlias);
}
- private static class ColumnFamilyRef {
+ protected void rememberTable(TableRef tableRef, String alias) {
+ String name = tableRef.getTable().getName().getString();
+ if (alias != null) {
+ tableMap.put(alias, tableRef);
+ } else {
+ tableMap.put(name, tableRef);
+ }
+ tables.add(tableRef);
+ }
+
+ protected static class ColumnFamilyRef {
Review comment:
This can be private?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1121,46 +1124,55 @@ public PSchema resolveSchema(String schemaName) throws
SQLException {
return null;
}
}
-
+
private static class ProjectedTableColumnResolver extends
MultiTableColumnResolver {
private final boolean isLocalIndex;
+ // We must handle the local index data tables separately
+ protected final ListMultimap<String, TableRef> localIndexDataTableMap
= ArrayListMultimap.<String, TableRef> create();
+ protected final List<TableRef> localIndexDataTables = new
ArrayList<>();;
private final List<TableRef> theTableRefs;
private final Map<ColumnRef, Integer> columnRefMap;
private ProjectedTableColumnResolver(PTable projectedTable,
PhoenixConnection conn, Map<String, UDFParseNode> udfParseNodes) throws
SQLException {
+
super(conn, 0, udfParseNodes, null);
Preconditions.checkArgument(projectedTable.getType() ==
PTableType.PROJECTED);
this.isLocalIndex = projectedTable.getIndexType() ==
IndexType.LOCAL;
this.columnRefMap = new HashMap<ColumnRef, Integer>();
long ts = Long.MAX_VALUE;
for (int i = projectedTable.getBucketNum() == null ? 0 : 1; i <
projectedTable.getColumns().size(); i++) {
PColumn column = projectedTable.getColumns().get(i);
- ColumnRef colRef = ((ProjectedColumn)
column).getSourceColumnRef();
- TableRef tableRef = colRef.getTableRef();
- if (!tables.contains(tableRef)) {
- String alias = tableRef.getTableAlias();
- if (alias != null) {
- this.tableMap.put(alias, tableRef);
- }
- String name = tableRef.getTable().getName().getString();
- if (alias == null || !alias.equals(name)) {
- tableMap.put(name, tableRef);
- }
- tables.add(tableRef);
- if (tableRef.getLowerBoundTimeStamp() < ts) {
- ts = tableRef.getLowerBoundTimeStamp();
- }
+ ColumnRef sourceColRef = ((ProjectedColumn)
column).getSourceColumnRef();
+ TableRef tableRef = sourceColRef.getTableRef();
+
+ if (sourceColRef instanceof LocalIndexDataColumnRef &&
!localIndexDataTables.contains(tableRef)) {
+ rememberLocalIndexDataTable(tableRef,
tableRef.getTableAlias());
+ } else
+ if (!tables.contains(tableRef)) {
+ rememberTable(tableRef, tableRef.getTableAlias());
+ }
+ if (tableRef.getLowerBoundTimeStamp() < ts) {
+ ts = tableRef.getLowerBoundTimeStamp();
}
- this.columnRefMap.put(new ColumnRef(tableRef,
colRef.getColumnPosition()), column.getPosition());
+ this.columnRefMap.put(new ColumnRef(tableRef,
sourceColRef.getColumnPosition()), column.getPosition());
}
this.theTableRefs = ImmutableList.of(new
TableRef(ParseNodeFactory.createTempAlias(), projectedTable, ts, false));
-
}
-
+
+ protected void rememberLocalIndexDataTable(TableRef tableRef, String
alias) {
+ String name = tableRef.getTable().getName().getString();
Review comment:
nit: similar to `rememberTable`, this can also be put inside else block
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1171,14 +1183,69 @@ public ColumnRef resolveColumn(String schemaName,
String tableName, String colNa
TableRef tableRef = isLocalIndex ? super.getTables().get(0) :
super.resolveTable(schemaName, tableName);
if (tableRef.getTable().getIndexType() == IndexType.LOCAL) {
try {
- TableRef parentTableRef = super.resolveTable(
-
tableRef.getTable().getSchemaName().getString(),
-
tableRef.getTable().getParentTableName().getString());
+ TableRef parentTableRef = null;
+ if (tableName == null) {
+ // No table specified
+ Iterator<TableRef> iterator =
localIndexDataTables.iterator();
+ while (iterator.hasNext()) {
+ TableRef searchTableRef = iterator.next();
+ try {
+
searchTableRef.getTable().getColumnForColumnName(colName);
+ if (parentTableRef != null) {
+ throw new
AmbiguousColumnException(colName);
+ }
+ parentTableRef = searchTableRef;
+ } catch (ColumnNotFoundException e2) {
+ }
+ }
+ if (parentTableRef == null) {
+ throw new ColumnNotFoundException(schemaName,
tableName, null, colName);
+ }
+ } else {
+ // table specified
+ parentTableRef = resolveLocalIndexDataTable(
schemaName, tableName);
+ }
colRef = new ColumnRef(parentTableRef,
IndexUtil.getDataColumnFamilyName(colName),
IndexUtil.getDataColumnName(colName));
} catch (TableNotFoundException te) {
- throw e;
+ //columfamily format
+ TableRef theTableRef = null;
+ PColumnFamily theColumnFamily = null;
+ PColumn theColumn = null;
+ if (schemaName != null) {
+ try {
+ // Try schemaName as the tableName and use
tableName as column family name
+ theTableRef = resolveLocalIndexDataTable(null,
schemaName);
+ theColumnFamily =
theTableRef.getTable().getColumnFamily(tableName);
+ theColumn =
theColumnFamily.getPColumnForColumnName(colName);
+ } catch (MetaDataEntityNotFoundException e2) {
+ }
+ }
+ if (theColumn == null) {
+ // Try using the tableName as a columnFamily
reference instead
+ // and resolve column in each column family.
+ Iterator<TableRef> iterator =
localIndexDataTables.iterator();
+ while (iterator.hasNext()) {
+ TableRef searchTableRef = iterator.next();
Review comment:
Same as above, could be replaced with `for (TableRef
localIndexDataTableRef : localIndexDataTables)`
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1171,14 +1183,69 @@ public ColumnRef resolveColumn(String schemaName,
String tableName, String colNa
TableRef tableRef = isLocalIndex ? super.getTables().get(0) :
super.resolveTable(schemaName, tableName);
if (tableRef.getTable().getIndexType() == IndexType.LOCAL) {
try {
- TableRef parentTableRef = super.resolveTable(
-
tableRef.getTable().getSchemaName().getString(),
-
tableRef.getTable().getParentTableName().getString());
+ TableRef parentTableRef = null;
+ if (tableName == null) {
+ // No table specified
+ Iterator<TableRef> iterator =
localIndexDataTables.iterator();
+ while (iterator.hasNext()) {
+ TableRef searchTableRef = iterator.next();
Review comment:
Could be clubbed to `for (TableRef localIndexDataTableRef :
localIndexDataTables)`
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -985,7 +976,17 @@ public TableRef refreshDerivedTableNode(DerivedTableNode
derivedTableNode) throw
return this.resolveTable(null, tableAlias);
}
- private static class ColumnFamilyRef {
+ protected void rememberTable(TableRef tableRef, String alias) {
+ String name = tableRef.getTable().getName().getString();
Review comment:
nit: could be inside `else` block
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/schema/LocalIndexDataColumnRef.java
##########
@@ -37,7 +37,7 @@ public LocalIndexDataColumnRef(StatementContext context,
TableRef tRef, String i
throws MetaDataEntityNotFoundException, SQLException {
super(FromCompiler.getResolver(
FACTORY.namedTable(
- null,
+ tRef.getTableAlias(),
Review comment:
is this a missing part even before this patch?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
##########
@@ -1171,14 +1183,69 @@ public ColumnRef resolveColumn(String schemaName,
String tableName, String colNa
TableRef tableRef = isLocalIndex ? super.getTables().get(0) :
super.resolveTable(schemaName, tableName);
if (tableRef.getTable().getIndexType() == IndexType.LOCAL) {
try {
- TableRef parentTableRef = super.resolveTable(
-
tableRef.getTable().getSchemaName().getString(),
-
tableRef.getTable().getParentTableName().getString());
+ TableRef parentTableRef = null;
+ if (tableName == null) {
+ // No table specified
+ Iterator<TableRef> iterator =
localIndexDataTables.iterator();
+ while (iterator.hasNext()) {
+ TableRef searchTableRef = iterator.next();
+ try {
+
searchTableRef.getTable().getColumnForColumnName(colName);
+ if (parentTableRef != null) {
+ throw new
AmbiguousColumnException(colName);
+ }
+ parentTableRef = searchTableRef;
+ } catch (ColumnNotFoundException e2) {
+ }
Review comment:
DEBUG log would be useful by any chance in debugging?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
##########
@@ -226,6 +226,7 @@ private static void projectAllIndexColumns(StatementContext
context, TableRef ta
String indexColName = IndexUtil.getIndexColumnName(tableColumn);
PColumn indexColumn = null;
ColumnRef ref = null;
+ boolean localIndexResolved = true;
Review comment:
This should be `false`? Otherwise, this logic in catch block will never
be true:
```
if(!localIndexResolved) {
throw e;
}
```
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
##########
@@ -701,8 +723,7 @@ public Void visit(ProjectedColumnExpression expression) {
indexProjectedColumns.add(expression);
PColumn col = expression.getColumn();
// hack'ish... For covered columns with local
indexes we defer to the server.
- if (col instanceof ProjectedColumn &&
((ProjectedColumn) col)
- .getSourceColumnRef() instanceof
LocalIndexDataColumnRef) {
+ if (context.getDataColumns().contains(col)) {
Review comment:
Could you please explain this change? I am not sure about this 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> Bogus AmbiguousTableException in query with aliases on local indexed tables
> ---------------------------------------------------------------------------
>
> Key: PHOENIX-6365
> URL: https://issues.apache.org/jira/browse/PHOENIX-6365
> Project: Phoenix
> Issue Type: Task
> Components: core
> Affects Versions: 5.1.0, 4.16.0
> Reporter: Istvan Toth
> Assignee: Istvan Toth
> Priority: Major
>
> Certain queries with aliases on tbales with local indexes throw
> AmbiguousTableException
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
