[Bitsail] Refactor Oracle connector and add unit tests on util classes #82
Conversation
|
@ysamchu the unit tests are failing. |
I checked the test log. It fails on maven dependencies and seems not related to my change. Rerun the unit test and pass https://github.com/bytedance/bitsail/actions/runs/3374216700/jobs/5601154392. |
| @@ -369,10 +364,10 @@ protected Double convertDoubleValue(Object value, String columnName, String colu | |||
| return ((BigDecimal) value).doubleValue(); | |||
| } | |||
| throw new IllegalArgumentException(String | |||
| .format("Unexpected value for JDBC type: %s and column %s", columnTypeName, columnName)); | |||
There was a problem hiding this comment.
This debug message is quite useful to local the column that has an issue. Is there any reason we wanna make this change?
There was a problem hiding this comment.
The reason I made this change was because I didn't see a logic usage besides logging for columnName and columnTypeName. However it makes sense if we want those specific logs to easily locate issue. Will add columnName and columnTypeName back.
One follow-up question for this log: Original logging doesn't printout value. Is there any reason (such as privacy concern) for doing so? If no then I can printout value to help debugging.
There was a problem hiding this comment.
We don't have a standard whether to log value or not, some connectors does.
|
Overall LGTM~ |
ysamchu
force-pushed
the
refactor-oracle-connector
branch
from
2f94fe3
to
c196e19
Compare
Signed-off-by:
Pre-Checklist
Note: Please complete ALL items in the following checklist.
Purpose
Refactor Oracle connector and add unit tests on util classes
Approaches
columnNamethroughout JdbcValueConverter and its children classes.convertIntervalDSValue()&convertIntervalYMValue()=>convertInterval()Related Issues
N/A
New Behavior (screenshots if needed)
N/A