iamhucong commented on PR #38897:
URL: https://github.com/apache/shardingsphere/pull/38897#issuecomment-4778065822

   ### Review Result: Mergeable
   
   The change looks good to me.
   
   This PR fixes the shared insert-value SQL rendering path directly. 
`TemporalLiteralExpressionSegment` extends `LiteralExpressionSegment`, but when 
`InsertValue` rebuilds `INSERT VALUES` SQL, temporal literals need to keep 
their original SQL text. Otherwise a value such as `DATE '2017-08-08'` can be 
rendered as an ordinary string literal.
   
   The new check returns `TemporalLiteralExpressionSegment#getText()` before 
the generic literal formatter, while keeping the existing behavior for 
parameter markers, normal literals, and other expressions.
   
   The added `InsertValueTest` coverage for `(DATE '2017-08-08')` matches the 
changed behavior. I also do not see a need for release notes or user docs 
because this is a narrow internal rewrite correctness fix without API, 
configuration, or migration impact.
   
   Verification:
   
   - `./mvnw -pl infra/rewrite/core -am -DskipITs -Dspotless.skip=true 
-Dtest=InsertValueTest -Dsurefire.failIfNoSpecifiedTests=false test`
   - `./mvnw spotless:apply -Pcheck -T1C`
   - `./mvnw checkstyle:check -Pcheck -T1C`
   


-- 
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]

Reply via email to