stoty commented on code in PR #1665:
URL: https://github.com/apache/phoenix/pull/1665#discussion_r1319836291
##########
phoenix-core/src/test/java/org/apache/phoenix/compile/QueryMetaDataTest.java:
##########
@@ -309,7 +309,7 @@ public void testBasicResultSetMetaData() throws Exception {
assertEquals("organization_id".toUpperCase(),md.getColumnName(1));
assertEquals("a_string".toUpperCase(),md.getColumnName(2));
assertEquals("b_string".toUpperCase(),md.getColumnName(3));
- assertEquals("i".toUpperCase(),md.getColumnName(4));
+ assertEquals("a_integer".toUpperCase(),md.getColumnName(4));
Review Comment:
check the alias as well.
##########
phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionProjector.java:
##########
@@ -39,14 +39,24 @@ public class ExpressionProjector implements ColumnProjector
{
private final Expression expression;
private final String tableName;
private final boolean isCaseSensitive;
-
+ private final String label;
+
+ public ExpressionProjector(String name, String tableName, Expression
expression, boolean isCaseSensitive, String label) {
+ this.name = name;
+ this.expression = expression;
+ this.tableName = tableName;
+ this.isCaseSensitive = isCaseSensitive;
+ this.label = label;
+ }
+
public ExpressionProjector(String name, String tableName, Expression
expression, boolean isCaseSensitive) {
Review Comment:
What is the resaon for keeping this ?
##########
phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionProjector.java:
##########
@@ -39,14 +39,24 @@ public class ExpressionProjector implements ColumnProjector
{
private final Expression expression;
private final String tableName;
private final boolean isCaseSensitive;
-
+ private final String label;
+
+ public ExpressionProjector(String name, String tableName, Expression
expression, boolean isCaseSensitive, String label) {
Review Comment:
nit: name and label are strongly related, I'd prefer if they were
consecutive parameters.
##########
phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionProjector.java:
##########
@@ -39,14 +39,24 @@ public class ExpressionProjector implements ColumnProjector
{
private final Expression expression;
private final String tableName;
private final boolean isCaseSensitive;
-
+ private final String label;
+
+ public ExpressionProjector(String name, String tableName, Expression
expression, boolean isCaseSensitive, String label) {
+ this.name = name;
+ this.expression = expression;
+ this.tableName = tableName;
+ this.isCaseSensitive = isCaseSensitive;
+ this.label = label;
+ }
+
public ExpressionProjector(String name, String tableName, Expression
expression, boolean isCaseSensitive) {
Review Comment:
Having a single construnctor which requires all parameters would make it
harder to introduce new bugs.
##########
phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java:
##########
@@ -470,10 +470,24 @@ public static RowProjector compile(StatementContext
context, SelectStatement sta
ExpressionCompiler.throwNonAggExpressionInAggException(expression.toString());
}
}
- String columnAlias = aliasedNode.getAlias() != null ?
aliasedNode.getAlias() :
SchemaUtil.normalizeIdentifier(aliasedNode.getNode().getAlias());
- boolean isCaseSensitive = aliasedNode.getAlias() != null ?
aliasedNode.isCaseSensitve() : (columnAlias != null ?
SchemaUtil.isCaseSensitive(aliasedNode.getNode().getAlias()) :
selectVisitor.isCaseSensitive);
- String name = columnAlias == null ? expression.toString() :
columnAlias;
- projectedColumns.add(new ExpressionProjector(name,
tableRef.getTableAlias() == null ? (table.getName() == null ? "" :
table.getName().getString()) : tableRef.getTableAlias(), expression,
isCaseSensitive));
+
+ String tableName = tableRef.getTableAlias() == null ?
+ (table.getName() == null ?
+ "" :
+ table.getName().getString()) :
+ tableRef.getTableAlias();
+ String colName =
SchemaUtil.normalizeIdentifier(aliasedNode.getNode().getAlias());
Review Comment:
Does this work because the the getAlias() here is not the alias set in
select command, and is only set for direct columns where we omit the column
family ?
##########
phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixResultSetMetadataTest.java:
##########
@@ -49,4 +51,80 @@ public void testNullTypeName() throws Exception {
assertEquals("NULL", rs.getMetaData().getColumnTypeName(1));
}
+
+ @Test
+ public void testCaseSensitiveExpression() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl());
+ conn.createStatement().execute(
+ "CREATE TABLE T (pk1 CHAR(15) not null, pk2 VARCHAR not null,
\"v1\" VARCHAR(15), v2 DATE, \"v3\" VARCHAR " +
+ "CONSTRAINT pk PRIMARY KEY (pk1, pk2)) ");
+ ResultSet rs = conn.createStatement().executeQuery("SELECT pk1 AS
testalias1, pk2 AS \"testalias2\", " +
+ "\"v1\" AS \"testalias3\", v2, \"v3\" FROM T");
+
+ assertEquals("PK1", rs.getMetaData().getColumnName(1));
+ assertEquals("TESTALIAS1", rs.getMetaData().getColumnLabel(1));
+ assertFalse(rs.getMetaData().isCaseSensitive(1));
+
+ assertEquals("PK2", rs.getMetaData().getColumnName(2));
+ assertEquals("testalias2", rs.getMetaData().getColumnLabel(2));
+ assertTrue(rs.getMetaData().isCaseSensitive(2));
+
+ assertEquals("v1", rs.getMetaData().getColumnName(3));
+ assertEquals("testalias3", rs.getMetaData().getColumnLabel(3));
+ assertTrue(rs.getMetaData().isCaseSensitive(3));
+
+ assertEquals("V2", rs.getMetaData().getColumnName(4));
+ assertEquals("V2", rs.getMetaData().getColumnLabel(4));
+ assertFalse(rs.getMetaData().isCaseSensitive(4));
+
+ assertEquals("v3", rs.getMetaData().getColumnName(5));
+ assertEquals("v3", rs.getMetaData().getColumnLabel(5));
+ assertTrue(rs.getMetaData().isCaseSensitive(5));
+ }
+
+ @Test
Review Comment:
Can you add a separate test for the VIEW case that was found by
BackwardCompatibilityIT ?
--
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]
