Add Oracle SQL - Update statement

[ThanoshanMV]

#10111

Hi @wgy8283335, I've added SQL definition for Oracle UPDATE statement. Please check it.

Changes proposed in this pull request:

• Add Oracle DELETE statement.
• Add test cases.
• I commented SQL case id update_with_special_comments as it's not aligned with Oracle SQL's returningCluase:

• As Oracle allows us to set a value to a list of columns, I tried to get the list of column values in the AssignmentSegment:

[codecov-commenter]

Codecov Report

Merging #11692 (7fba441) into master (0ccdc93) will increase coverage by 0.03%.
The diff coverage is 50.38%.

@@             Coverage Diff              @@
##             master   #11692      +/-   ##
============================================
+ Coverage     63.11%   63.15%   +0.03%     
  Complexity     1225     1225              
============================================
  Files          2328     2329       +1     
  Lines         35177    35263      +86     
  Branches       6130     6141      +11     
============================================
+ Hits          22203    22271      +68     
- Misses        11181    11196      +15     
- Partials       1793     1796       +3     

Impacted Files	Coverage Δ
...meter/impl/EncryptAssignmentParameterRewriter.java	0.00% <0.00%> (ø)
...enerator/impl/EncryptAssignmentTokenGenerator.java	0.00% <0.00%> (ø)
...ator/impl/EncryptInsertOnUpdateTokenGenerator.java	0.00% <0.00%> (ø)
...ql/parser/sql/common/extractor/TableExtractor.java	5.40% <0.00%> (ø)
...mmon/segment/dml/assignment/AssignmentSegment.java	0.00% <0.00%> (ø)
...d/asserts/segment/assignment/AssignmentAssert.java	0.00% <0.00%> (ø)
...erts/segment/assignment/AssignmentValueAssert.java	0.00% <0.00%> (ø)
...in/segment/impl/assignment/ExpectedAssignment.java	100.00% <ø> (ø)
...gment/impl/assignment/ExpectedAssignmentValue.java	100.00% <ø> (ø)
...meter/impl/ShadowUpdateValueParameterRewriter.java	33.33% <50.00%> (ø)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ccdc93...7fba441. Read the comment docs.

[tristaZero]

Hi @ThanoshanMV ,

Looks our final evaluation is on the way. Let us make it all the way through and win! ;-) @ThanoshanMV @wgy8283335

[wgy8283335]

What about combining the column and columns?
And change the value to list.

@RequiredArgsConstructor
@Getter
public final class AssignmentSegment implements SQLSegment {
    
    private final int startIndex;
    
    private final int stopIndex;
    
    private final List<ColumnSegment> columns;
    
    private final List<ExpressionSegment> values;
}

[ThanoshanMV]

What about combining the column and columns?

We can combine columns as a list but since the AssignmentSegment is used across the ShardingSphere project, we need to make changes in all AssignmentSegment occurrences. For example in EncryptAssignmentParameterRewriter:

What shall we do?

[ThanoshanMV]

And change the value to list

I think we don't have to create a list for value, as an assignment can have only one value:

Please let me know what do you think.

[wgy8283335]

Below is not reasonable:

@RequiredArgsConstructor
@Getter
public final class AssignmentSegment implements SQLSegment {
    
    private final int startIndex;
    
    private final int stopIndex;
    
    private final ColumnSegment column;

    private List<ColumnSegment> columns = new LinkedList<>();
    
    private final ExpressionSegment value;
}

But what shall we do in this situation? @jingshanglu @tristaZero

UPDATE employees a SET department_id = (SELECT department_id FROM departments WHERE location_id = '2100'), (salary, commission_pct) = (SELECT 1.1*AVG(salary), 1.5*AVG(commission_pct) FROM employees b WHERE a.department_id = b.department_id);

[jingshanglu]

@wgy8283335 @ThanoshanMV Maybe we can extract abstract class and combine these two idea.

public abstract class AssignmentSegment implements SQLSegment {
    
   List<ColumnSegment> getColumns();
   ExpressionSegment getValue();
}

@RequiredArgsConstructor
@Getter
public final class OracleAssignmentSegment extends AssignmentSegment {
    
   private final int startIndex;
    
    private final int stopIndex;

    private List<ColumnSegment> columns = new LinkedList<>();
    
    private final ExpressionSegment value;
}

SET department_id = (SELECT department_id FROM departments WHERE location_id = '2100')
expressed as
OracleAssignmentSegment{columns=[department_id],value=subquery}

(salary, commission_pct) = (SELECT 1.1*AVG(salary), 1.5*AVG(commission_pct) FROM employees b WHERE a.department_id = b.department_id)
expressed as 
OracleAssignmentSegment{columns=[salary, commission_pct],value=subquery}

Maybe it has better decoupling and scalability, but we need to make changes in all AssignmentSegment occurrences.

[ThanoshanMV]

@jingshanglu do you mean like this:

AssignmentSegment:

OracleAssignmentSegment:

[ThanoshanMV]

Shall we create similar child classes for other SQL dialects, for example, PostgreSQLAssignmentSegment or shall we rename OracleAssignmentSegment to a generic one?

Currently, whenever we need to create an instance of AssignmentSegment, we have to utilize OracleAssignmentSegment. In the below image, the method visitSetClause is in PostgreSQLStatementSQLVisitor which returns an instance of AssignmentSegment:

[jingshanglu]

Shall we create similar child classes for other SQL dialects, for example, PostgreSQLAssignmentSegment or shall we rename OracleAssignmentSegment to a generic one?

Currently, whenever we need to create an instance of AssignmentSegment, we have to utilize OracleAssignmentSegment. In the below image, the method visitSetClause is in PostgreSQLStatementSQLVisitor which returns an instance of AssignmentSegment:

@ThanoshanMV Yes, maybe it is better to rename OracleAssignmentSegment to a genneric one, define new subclasses when we need to expand.

[wgy8283335]

Is it need to be initialized with LinkedList, as ColumnAssignmentSegment has @RequiredArgsConstructor?

[ThanoshanMV]

I'll change as private final List<ColumnSegment> columns, remove the initialization and make other changes accordingly.

[wgy8283335]

There are many changes in this pr, and it's better to make all of the ci checks pass.

[tristaZero]

Hi @wgy8283335 Thanks for your tips, all the CI tests passed, do you think this one is ready to merge?

[wgy8283335]

@ThanoshanMV, have you finished your modification?

[ThanoshanMV]

@ThanoshanMV, have you finished your modification?

Yes @wgy8283335. I changed as private final List<ColumnSegment> columns and removed commented SQL case id update_with_special_comments as it's not aligned with Oracle SQL's returningCluase.