----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17642/#review34271 -----------------------------------------------------------
Hi Jihoon, I reviewed the patch. This is nice work. I've wanted this feature. This patch includes the use of qualified asterisk. So, if you add more unit tests using qualified asterisks following, the patch would be nice. > select t1.*, t2.* from t1, t2; > select t2.*, t1.* from t1, t2; > select t1.*, t1.a as name1, t2.* from t1, t2; > select t1.a as name1, t1.*, t1.b as name2, t2.* from t1, t2; ... (more combinations if needed) Thank you! tajo-algebra/src/main/java/org/apache/tajo/algebra/AsteriskedQualifierExpr.java <https://reviews.apache.org/r/17642/#comment64299> I think that QualifiedAsteriskExpr is more proper to its meaning. It's because 'qualified' is used for that the column reference with a table name. We also can see an asterisk (*) as an expression to indicates all columns in all tables within a query block. So, I would like to suggest QualifiedAsteriskExpr. - Hyunsik Choi On Feb. 13, 2014, 11:29 a.m., Jihoon Son wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17642/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2014, 11:29 a.m.) > > > Review request for Tajo, Jung JaeHwa and Hyunsik Choi. > > > Bugs: TAJO-554 > https://issues.apache.org/jira/browse/TAJO-554 > > > Repository: tajo > > > Description > ------- > > See the title. LogicalPlanner should allow the following case. > > SELECT *, col1 + 10, col2 FROM ... > > In this patch, the asterisk expression is replaced with the target relation's > columns in LogicalPlanPreprocessor. > > > Diffs > ----- > > > tajo-algebra/src/main/java/org/apache/tajo/algebra/AsteriskedQualifierExpr.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/OpType.java 84f784e > tajo-algebra/src/main/java/org/apache/tajo/algebra/Projection.java 3d9f8a6 > > tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 > ab6bff6 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/HiveConverter.java > a8a555b > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java > 493f892 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/AlgebraVisitor.java > 1c710dc > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/BaseAlgebraVisitor.java > 25ee316 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java > 6363bf6 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java > 9890943 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java > 099e462 > > tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java > d929218 > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java > 81f57d4 > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSelectQuery.java > 4d852fc > > tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java > b6e439a > > tajo-core/tajo-core-backend/src/test/resources/queries/TestSelectQuery/testSelectAsterisk4.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testAsterisk.sql > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/results/TestSelectQuery/testSelectAsterisk4.result > PRE-CREATION > > tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testAsterisk.result > PRE-CREATION > > Diff: https://reviews.apache.org/r/17642/diff/ > > > Testing > ------- > > mvn verify > > > Thanks, > > Jihoon Son > >
