diff --git ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java index 9c39cb20bb..a3b3d259bb 100644 --- ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java +++ ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java @@ -3246,8 +3246,12 @@ private TableType obtainTableType(Table tabMetaData) { private RelNode genFilterRelNode(ASTNode filterExpr, RelNode srcRel, ImmutableMap outerNameToPosMap, RowResolver outerRR, boolean useCaching) throws SemanticException { - ExprNodeDesc filterCondn = genExprNodeDesc(filterExpr, relToHiveRR.get(srcRel), - outerRR, null, useCaching); + ExprNodeDesc filterCondn = genExprNodeDesc(filterExpr, relToHiveRR.get(srcRel), outerRR, null, useCaching); + return genFilterRelNode(filterCondn, srcRel, outerNameToPosMap, outerRR); + } + + private RelNode genFilterRelNode(ExprNodeDesc filterCondn, RelNode srcRel, + ImmutableMap outerNameToPosMap, RowResolver outerRR) throws SemanticException { if (filterCondn instanceof ExprNodeConstantDesc && !filterCondn.getTypeString().equals(serdeConstants.BOOLEAN_TYPE_NAME)) { // queries like select * from t1 where 'foo'; @@ -3519,6 +3523,28 @@ private RelNode genFilterLogicalPlan(QB qb, RelNode srcRel, ImmutableMap genConstraintFilterLogicalPlan( + QB qb, RelNode srcRel, ImmutableMap outerNameToPosMap, RowResolver outerRR) + throws SemanticException { + if (qb.getIsQuery()) { + return null; + } + + String dest = qb.getParseInfo().getClauseNames().iterator().next(); + if (!updating(dest)) { + return null; + } + + RowResolver inputRR = relToHiveRR.get(srcRel); + ExprNodeDesc constraintUDF = genConstraintsExpr(dest, qb, inputRR); + if (constraintUDF == null) { + return null; + } + + RelNode constraintRel = genFilterRelNode(constraintUDF, srcRel, outerNameToPosMap, outerRR); + return new Pair<>(constraintRel, inputRR); + } + /** * Class to store GenericUDAF related information. */ @@ -5164,6 +5190,13 @@ private RelNode genLogicalPlan(QB qb, boolean outerMostQB, selectRel = selPair.getKey(); srcRel = (selectRel == null) ? srcRel : selectRel; + // Build Rel for Constraint checks + Pair constraintPair = + genConstraintFilterLogicalPlan(qb, srcRel, outerNameToPosMap, outerRR); + if (constraintPair != null) { + selPair = constraintPair; + } + // 6. Build Rel for OB Clause obRel = genOBLogicalPlan(qb, selPair, outerMostQB); srcRel = (obRel == null) ? srcRel : obRel; diff --git ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java index c87f2d2292..df798144d4 100644 --- ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java +++ ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java @@ -7109,7 +7109,8 @@ private void replaceColumnReference(ASTNode checkExpr, Map col2C } } - private ExprNodeDesc getCheckConstraintExpr(Table tbl, Operator input, RowResolver inputRR, String dest) + private ExprNodeDesc getCheckConstraintExpr( + Table tbl, List inputColInfos, RowResolver inputRR, String dest) throws SemanticException{ CheckConstraint cc = null; @@ -7127,7 +7128,6 @@ private ExprNodeDesc getCheckConstraintExpr(Table tbl, Operator input, RowResolv // this will be used to replace column references in CHECK expression AST with corresponding column name // in input Map col2Cols = new HashMap<>(); - List colInfos = input.getSchema().getSignature(); int colIdx = 0; if(updating(dest)) { // if this is an update we need to skip the first col since it is row id @@ -7136,7 +7136,7 @@ private ExprNodeDesc getCheckConstraintExpr(Table tbl, Operator input, RowResolv for(FieldSchema fs: tbl.getCols()) { // since SQL is case insenstive just to make sure that the comparison b/w column names // and check expression's column reference work convert the key to lower case - col2Cols.put(fs.getName().toLowerCase(), colInfos.get(colIdx).getInternalName()); + col2Cols.put(fs.getName().toLowerCase(), inputColInfos.get(colIdx).getInternalName()); colIdx++; } @@ -7206,17 +7206,7 @@ private boolean mergeCardinalityViolationBranch(final Operator input) { return false; } - private Operator genConstraintsPlan(String dest, QB qb, Operator input) throws SemanticException { - if(deleting(dest)) { - // for DELETE statements NOT NULL constraint need not be checked - return input; - } - - //MERGE statements could have inserted a cardinality violation branch, we need to avoid that - if(mergeCardinalityViolationBranch(input)){ - return input; - } - + protected ExprNodeDesc genConstraintsExpr(String dest, QB qb, RowResolver inputRR) throws SemanticException { // if this is an insert into statement we might need to add constraint check Table targetTable = null; Integer dest_type = qb.getMetaData().getDestTypeForAlias(dest); @@ -7233,13 +7223,12 @@ else if(dest_type == QBMetaData.DEST_PARTITION){ throw new SemanticException("Generating constraint check plan: Invalid target type: " + dest); } - RowResolver inputRR = opParseCtx.get(input).getRowResolver(); - ExprNodeDesc nullConstraintExpr = getNotNullConstraintExpr(targetTable, input, dest); - ExprNodeDesc checkConstraintExpr = getCheckConstraintExpr(targetTable, input, inputRR, dest); + List inputColumnInfoList = inputRR.getColumnInfos(); + ExprNodeDesc nullConstraintExpr = getNotNullConstraintExpr(targetTable, inputColumnInfoList, dest); + ExprNodeDesc checkConstraintExpr = getCheckConstraintExpr(targetTable, inputColumnInfoList, inputRR, dest); ExprNodeDesc combinedConstraintExpr = null; if(nullConstraintExpr != null && checkConstraintExpr != null) { - assert (input.getParentOperators().size() == 1); combinedConstraintExpr = ExprNodeTypeCheck.getExprNodeDefaultExprProcessor() .getFuncExprNodeDesc("and", nullConstraintExpr, checkConstraintExpr); @@ -7251,17 +7240,37 @@ else if(checkConstraintExpr != null) { combinedConstraintExpr = checkConstraintExpr; } + if (combinedConstraintExpr == null) { + return null; + } + return ExprNodeTypeCheck.getExprNodeDefaultExprProcessor() + .getFuncExprNodeDesc("enforce_constraint", combinedConstraintExpr); + } + + private Operator genConstraintsPlan(String dest, QB qb, Operator input) throws SemanticException { + if(deleting(dest)) { + // for DELETE statements NOT NULL constraint need not be checked + return input; + } + + //MERGE statements could have inserted a cardinality violation branch, we need to avoid that + if(mergeCardinalityViolationBranch(input)){ + return input; + } + + assert (input.getParentOperators().size() == 1); + RowResolver inputRR = opParseCtx.get(input).getRowResolver(); + ExprNodeDesc combinedConstraintExpr = genConstraintsExpr(dest, qb, inputRR); if (combinedConstraintExpr != null) { - ExprNodeDesc constraintUDF = ExprNodeTypeCheck.getExprNodeDefaultExprProcessor() - .getFuncExprNodeDesc("enforce_constraint", combinedConstraintExpr); return putOpInsertMap(OperatorFactory.getAndMakeChild( - new FilterDesc(constraintUDF, false), new RowSchema( + new FilterDesc(combinedConstraintExpr, false), new RowSchema( inputRR.getColumnInfos()), input), inputRR); } return input; } - private ExprNodeDesc getNotNullConstraintExpr(Table targetTable, Operator input, String dest) throws SemanticException { + private ExprNodeDesc getNotNullConstraintExpr(Table targetTable, List inputColInfos, String dest) + throws SemanticException { boolean forceNotNullConstraint = conf.getBoolVar(ConfVars.HIVE_ENFORCE_NOT_NULL_CONSTRAINT); if(!forceNotNullConstraint) { return null; @@ -7281,18 +7290,17 @@ private ExprNodeDesc getNotNullConstraintExpr(Table targetTable, Operator input, if(nullConstraintBitSet == null) { return null; } - List colInfos = input.getSchema().getSignature(); ExprNodeDesc currUDF = null; // Add NOT NULL constraints int constraintIdx = 0; - for(int colExprIdx=0; colExprIdx < colInfos.size(); colExprIdx++) { + for(int colExprIdx=0; colExprIdx < inputColInfos.size(); colExprIdx++) { if(updating(dest) && colExprIdx == 0) { // for updates first column is _rowid continue; } if (nullConstraintBitSet.indexOf(constraintIdx) != -1) { - ExprNodeDesc currExpr = ExprNodeTypeCheck.toExprNodeDesc(colInfos.get(colExprIdx)); + ExprNodeDesc currExpr = ExprNodeTypeCheck.toExprNodeDesc(inputColInfos.get(colExprIdx)); ExprNodeDesc isNotNullUDF = ExprNodeTypeCheck.getExprNodeDefaultExprProcessor() .getFuncExprNodeDesc("isnotnull", currExpr); if (currUDF != null) { @@ -15040,7 +15048,7 @@ private boolean isAcidOutputFormat(Class of) { } } - private boolean updating(String destination) { + protected boolean updating(String destination) { return destination.startsWith(Context.DestClausePrefix.UPDATE.toString()); } diff --git ql/src/test/results/clientnegative/update_notnull_constraint.q.out ql/src/test/results/clientnegative/update_notnull_constraint.q.out index 32905378e7..86bfc67480 100644 --- ql/src/test/results/clientnegative/update_notnull_constraint.q.out +++ ql/src/test/results/clientnegative/update_notnull_constraint.q.out @@ -21,9 +21,4 @@ POSTHOOK: Output: default@acid_uami POSTHOOK: Lineage: acid_uami.de SCRIPT [] POSTHOOK: Lineage: acid_uami.i SCRIPT [] POSTHOOK: Lineage: acid_uami.vc SCRIPT [] -PREHOOK: query: UPDATE acid_uami set de=null where i=1 -PREHOOK: type: QUERY -PREHOOK: Input: default@acid_uami -PREHOOK: Output: default@acid_uami -#### A masked pattern was here #### -FAILED: Execution Error, return code 2 from org.apache.hadoop.hive.ql.exec.mr.MapRedTask +FAILED: DataConstraintViolationError org.apache.hadoop.hive.ql.exec.errors.DataConstraintViolationError: Either CHECK or NOT NULL constraint violated! diff --git ql/src/test/results/clientpositive/llap/check_constraint.q.out ql/src/test/results/clientpositive/llap/check_constraint.q.out index 3ef0744c7b..d98befd4d1 100644 --- ql/src/test/results/clientpositive/llap/check_constraint.q.out +++ ql/src/test/results/clientpositive/llap/check_constraint.q.out @@ -2070,10 +2070,10 @@ STAGE PLANS: Map Operator Tree: TableScan alias: acid_uami_n0 - filterExpr: (de) IN (103, 119) (type: boolean) + filterExpr: ((de) IN (103, 119) and enforce_constraint((893.14 >= CAST( i AS decimal(5,2))) is not false)) (type: boolean) Statistics: Num rows: 1 Data size: 328 Basic stats: COMPLETE Column stats: NONE Filter Operator - predicate: (de) IN (103, 119) (type: boolean) + predicate: ((de) IN (103, 119) and enforce_constraint((893.14 >= CAST( i AS decimal(5,2))) is not false)) (type: boolean) Statistics: Num rows: 1 Data size: 328 Basic stats: COMPLETE Column stats: NONE Select Operator expressions: ROW__ID (type: struct), i (type: int), vc (type: varchar(128)) @@ -2095,18 +2095,15 @@ STAGE PLANS: expressions: KEY.reducesinkkey0 (type: struct), VALUE._col0 (type: int), 893.14 (type: decimal(5,2)), VALUE._col1 (type: varchar(128)) outputColumnNames: _col0, _col1, _col2, _col3 Statistics: Num rows: 1 Data size: 328 Basic stats: COMPLETE Column stats: NONE - Filter Operator - predicate: enforce_constraint((_col2 is not null and (_col2 >= CAST( _col1 AS decimal(5,2))) is not false)) (type: boolean) + File Output Operator + compressed: false Statistics: Num rows: 1 Data size: 328 Basic stats: COMPLETE Column stats: NONE - File Output Operator - compressed: false - Statistics: Num rows: 1 Data size: 328 Basic stats: COMPLETE Column stats: NONE - table: - input format: org.apache.hadoop.hive.ql.io.orc.OrcInputFormat - output format: org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat - serde: org.apache.hadoop.hive.ql.io.orc.OrcSerde - name: default.acid_uami_n0 - Write Type: UPDATE + table: + input format: org.apache.hadoop.hive.ql.io.orc.OrcInputFormat + output format: org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat + serde: org.apache.hadoop.hive.ql.io.orc.OrcSerde + name: default.acid_uami_n0 + Write Type: UPDATE Stage: Stage-2 Dependency Collection @@ -2202,25 +2199,18 @@ STAGE PLANS: Execution mode: vectorized, llap Reduce Operator Tree: Select Operator - expressions: KEY.reducesinkkey0 (type: struct), VALUE._col0 (type: int), 893.14 (type: decimal(5,2)), 'apache_hive' (type: string) + expressions: KEY.reducesinkkey0 (type: struct), VALUE._col0 (type: int), 893.14 (type: decimal(5,2)), 'apache_hive' (type: varchar(128)) outputColumnNames: _col0, _col1, _col2, _col3 Statistics: Num rows: 1 Data size: 116 Basic stats: COMPLETE Column stats: NONE - Filter Operator - predicate: enforce_constraint(_col2 is not null) (type: boolean) + File Output Operator + compressed: false Statistics: Num rows: 1 Data size: 116 Basic stats: COMPLETE Column stats: NONE - Select Operator - expressions: _col0 (type: struct), _col1 (type: int), _col2 (type: decimal(5,2)), CAST( _col3 AS varchar(128)) (type: varchar(128)) - outputColumnNames: _col0, _col1, _col2, _col3 - Statistics: Num rows: 1 Data size: 116 Basic stats: COMPLETE Column stats: NONE - File Output Operator - compressed: false - Statistics: Num rows: 1 Data size: 116 Basic stats: COMPLETE Column stats: NONE - table: - input format: org.apache.hadoop.hive.ql.io.orc.OrcInputFormat - output format: org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat - serde: org.apache.hadoop.hive.ql.io.orc.OrcSerde - name: default.acid_uami_n0 - Write Type: UPDATE + table: + input format: org.apache.hadoop.hive.ql.io.orc.OrcInputFormat + output format: org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat + serde: org.apache.hadoop.hive.ql.io.orc.OrcSerde + name: default.acid_uami_n0 + Write Type: UPDATE Stage: Stage-2 Dependency Collection