[CARBONDATA-3928] Removed records from exception message. #3921
Conversation
Why is this PR needed?
Currently, when the string length exceeds 32000 and bad record action as default then
records are thrown in exception message which violates security concern.
What changes were proposed in this PR?
Removed the records from exception message and only printed in logger file and redirected csv file.
Does this PR introduce any user interface change?
No
Is any new testcase added?
Yes
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4046/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2308/ |
| @@ -119,10 +118,6 @@ public CarbonRow convert(CarbonRow row) throws CarbonDataLoadingException { | |||
| .getTableProperties(); | |||
| String spatialProperty = properties.get(CarbonCommonConstants.SPATIAL_INDEX); | |||
| boolean isSpatialColumn = false; | |||
| Object[] rawData = row.getRawData(); | |||
| if (rawData == null) { | |||
| rawData = row.getData() == null ? null : row.getData().clone(); | |||
There was a problem hiding this comment.
Please revert back this change, somecases rawData will not be set, hence they are setting here before converting the row.
Now your current code will use null instead of row.getData().clone() in this scenario.
There was a problem hiding this comment.
I had only added this scenario last time because at that time we wanted rawdata for every bad record action. But now we need this only in case of bad record logger is enable or action is redirect. And in this case, rawdata will be always available.
|
LGTM |
Why is this PR needed?
Currently, when the string length exceeds 32000 and bad record action as fail then
records are thrown in exception message which violates security concern.
What changes were proposed in this PR?
Removed the records from the exception message and only printed in logger file and redirected csv file.
Does this PR introduce any user interface change?
Is any new testcase added?