Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-09 Thread via GitHub


LakshSingla merged PR #16130:
URL: https://github.com/apache/druid/pull/16130


-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


github-advanced-security[bot] commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551290113


##
processing/src/main/java/org/apache/druid/frame/write/RowBasedFrameWriter.java:
##
@@ -299,11 +300,15 @@
   .column(signature.getColumnName(i))
   .build();
   }
+  catch (ParseException pe) {
+throw Throwables.propagate(pe);

Review Comment:
   ## Deprecated method or constructor invocation
   
   Invoking [Throwables.propagate](1) should be avoided because it has been 
deprecated.
   
   [Show more 
details](https://github.com/apache/druid/security/code-scanning/7226)



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551132601


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.msq.indexing.error;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import org.apache.druid.java.util.common.StringUtils;
+
+import javax.annotation.Nullable;
+import java.util.Objects;
+
+@JsonTypeName(FrameFieldWriterFault.CODE)
+public class FrameFieldWriterFault extends BaseMSQFault
+{
+  static final String CODE = "FieldWriterError";

Review Comment:
   Went with your suggestion. We do have `UnknownFault`->`UnknownError` though, 
but `InvalidField` sounded fine.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551129181


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##
@@ -2983,6 +2984,20 @@ private MSQErrorReport 
mapQueryColumnNameToOutputColumnName(
   .build(),
   task.getQuerySpec().getColumnMappings()
   );
+} else if (workerErrorReport.getFault() instanceof 
FrameFieldWriterException) {
+  FrameFieldWriterException ffre = (FrameFieldWriterException) 
workerErrorReport.getFault();
+  return MSQErrorReport.fromException(
+  workerErrorReport.getTaskId(),
+  workerErrorReport.getHost(),
+  workerErrorReport.getStageNumber(),
+  FrameFieldWriterException.builder()
+   .source(ffre.getSource())
+   .rowNumber(ffre.getRowNumber())
+   .column(ffre.getColumn())
+   .errorMsg(ffre.getErrorMsg())

Review Comment:
   The field value isn't available at the place the exception starts -- which 
is `RowBasedFrameWriter#writeDataUsingFieldWriters`



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551124120


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##
@@ -49,7 +51,7 @@ public FrameFieldWriterFault(
 super(
 CODE,
 StringUtils.format(
-"Error[%s] while writing frame for source[%s], rowNumber[%d] 
column[%s]",
+"Error[%s] while writing a field for source[%s], rowNumber[%d], 
column[%s].",

Review Comment:
   Now propagate the RE's message directly to the Fault class.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551086264


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##
@@ -2983,6 +2984,20 @@ private MSQErrorReport 
mapQueryColumnNameToOutputColumnName(
   .build(),
   task.getQuerySpec().getColumnMappings()
   );
+} else if (workerErrorReport.getFault() instanceof 
FrameFieldWriterException) {
+  FrameFieldWriterException ffre = (FrameFieldWriterException) 
workerErrorReport.getFault();
+  return MSQErrorReport.fromException(
+  workerErrorReport.getTaskId(),
+  workerErrorReport.getHost(),
+  workerErrorReport.getStageNumber(),
+  FrameFieldWriterException.builder()
+   .source(ffre.getSource())
+   .rowNumber(ffre.getRowNumber())
+   .column(ffre.getColumn())
+   .errorMsg(ffre.getErrorMsg())

Review Comment:
   We should also add the problematic field.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551084415


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##
@@ -49,7 +51,7 @@ public FrameFieldWriterFault(
 super(
 CODE,
 StringUtils.format(
-"Error[%s] while writing frame for source[%s], rowNumber[%d] 
column[%s]",
+"Error[%s] while writing a field for source[%s], rowNumber[%d], 
column[%s].",

Review Comment:
   nit: Duplicate error message as `FrameFieldWriterException` , can extract 
the format string out. It will make the updates easier. 
   



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551082323


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.msq.indexing.error;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import org.apache.druid.java.util.common.StringUtils;
+
+import javax.annotation.Nullable;
+import java.util.Objects;
+
+@JsonTypeName(FrameFieldWriterFault.CODE)
+public class FrameFieldWriterFault extends BaseMSQFault
+{
+  static final String CODE = "FieldWriterError";

Review Comment:
   This doesn't match the class name, can you please update the class as well? 
Also, wrt the naming convention of other faults, we don't have "Error" in the 
suffix. I think something like "InvalidFieldFault" might be more suitable, and 
the name could be "InvalidField". I am open to other naming conventions as 
well. 



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551071976


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##


Review Comment:
   Done. I've also added the description of the fault in `reference.md` and 
removed the term `frame` from the fault code and log message since that is 
user-facing.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-04 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1551065872


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##
@@ -2983,6 +2984,20 @@ private MSQErrorReport 
mapQueryColumnNameToOutputColumnName(
   .build(),
   task.getQuerySpec().getColumnMappings()
   );
+} else if (workerErrorReport.getFault() instanceof 
FrameFieldWriterException) {
+  FrameFieldWriterException ffre = (FrameFieldWriterException) 
workerErrorReport.getFault();
+  return MSQErrorReport.fromException(
+  workerErrorReport.getTaskId(),
+  workerErrorReport.getHost(),
+  workerErrorReport.getStageNumber(),
+  FrameFieldWriterException.builder()
+   .source(ffre.getSource())
+   .rowNumber(ffre.getRowNumber())
+   .column(ffre.getColumn())
+   .errorMsg(ffre.getErrorMsg())
+   .build(),

Review Comment:
   IIUC, the ask is to remove the builder from `FrameFieldWriterException`. But 
we do build it progressively in `ScanQueryFrameProcessor`.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-03 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1549763794


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java:
##


Review Comment:
   Let's add this class to `MSQFaultsSerdeTest` for completeness. 



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-04-03 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1549761358


##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##
@@ -2983,6 +2984,20 @@ private MSQErrorReport 
mapQueryColumnNameToOutputColumnName(
   .build(),
   task.getQuerySpec().getColumnMappings()
   );
+} else if (workerErrorReport.getFault() instanceof 
FrameFieldWriterException) {
+  FrameFieldWriterException ffre = (FrameFieldWriterException) 
workerErrorReport.getFault();
+  return MSQErrorReport.fromException(
+  workerErrorReport.getTaskId(),
+  workerErrorReport.getHost(),
+  workerErrorReport.getStageNumber(),
+  FrameFieldWriterException.builder()
+   .source(ffre.getSource())
+   .rowNumber(ffre.getRowNumber())
+   .column(ffre.getColumn())
+   .errorMsg(ffre.getErrorMsg())
+   .build(),

Review Comment:
   Let's use the normal constructor since we are using all the parameters in a 
single place. Invalid null byte exception has the builder because it populates 
different information at different places up the call stack.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-31 Thread via GitHub


gargvishesh commented on PR #16130:
URL: https://github.com/apache/druid/pull/16130#issuecomment-2029185932

   > * Can you please add a test case where the exception gets generated?
   
   Tried multiple routes incl insert via external inline tables and insert via 
joins, but an error isn't generated. Therefore not adding the test case. 
   
   > * Also, what does it mean for MSQ, should MSQ retry if it encounters such 
exceptions or not? It seems like retrying shouldn't help here, but per my 
understanding, MSQ will still treat errors like these as `UnknownFaults,` and 
will retry. We should update that logic in MSQ (check how we do it for 
`InvalidNullByte`)
   
   The conversion to `FrameFieldWriterFault` now puts it in the category of 
non-retryable errors. 


-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-31 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1545971957


##
processing/src/main/java/org/apache/druid/frame/write/FrameFieldWriterException.java:
##


Review Comment:
   Converting `FrameFieldWriterException` now to an `MSQFault` type



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-31 Thread via GitHub


gargvishesh commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1545971524


##
processing/src/main/java/org/apache/druid/frame/write/InvalidNullByteException.java:
##
@@ -53,17 +50,16 @@ private InvalidNullByteException(
   @Nullable final Integer position
   )
   {
-super(StringUtils.format(
+super(column, StringUtils.format(

Review Comment:
   Converting to MSQFault now for which the specific exception class is 
required. `InvalidNullByteException` now extends `RuntimeException` directly.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-21 Thread via GitHub


github-advanced-security[bot] commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1534284022


##
processing/src/main/java/org/apache/druid/frame/write/RowBasedFrameWriter.java:
##
@@ -299,11 +300,15 @@
   .column(signature.getColumnName(i))
   .build();
   }
+  catch (ParseException pe){
+throw Throwables.propagate(pe);

Review Comment:
   ## Deprecated method or constructor invocation
   
   Invoking [Throwables.propagate](1) should be avoided because it has been 
deprecated.
   
   [Show more 
details](https://github.com/apache/druid/security/code-scanning/7160)



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-19 Thread via GitHub


cryptoe commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1530055165


##
processing/src/main/java/org/apache/druid/frame/write/FrameFieldWriterException.java:
##


Review Comment:
   I think we can make a MSQ fault out of this. That way we donot have to 
adjust any retry logic. 



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-19 Thread via GitHub


cryptoe commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1530054071


##
processing/src/main/java/org/apache/druid/frame/write/InvalidNullByteException.java:
##
@@ -53,17 +50,16 @@ private InvalidNullByteException(
   @Nullable final Integer position
   )
   {
-super(StringUtils.format(
+super(column, StringUtils.format(

Review Comment:
   +1



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-15 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1525991508


##
processing/src/main/java/org/apache/druid/frame/write/FrameFieldWriterException.java:
##


Review Comment:
   I doubt that we need this class anywhere. Can't we use a generic 
`RuntimeException` with the proper error message? Or better yet, use 
`DruidException,` so that we can analyze the message. 



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-15 Thread via GitHub


LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1525991508


##
processing/src/main/java/org/apache/druid/frame/write/FrameFieldWriterException.java:
##


Review Comment:
   I doubt that we need this class anywhere. Can't we use a generic 
`RuntimeException` with the proper error message? Or better yet, use 
DruidException, so that we can analyze the message. 



##
processing/src/main/java/org/apache/druid/frame/write/InvalidNullByteException.java:
##
@@ -53,17 +50,16 @@ private InvalidNullByteException(
   @Nullable final Integer position
   )
   {
-super(StringUtils.format(
+super(column, StringUtils.format(

Review Comment:
   Feels weird that we have this extra layer of indirection, to what will 
essentially call the `RuntimeException`. If we don't extract the relevant 
fields (`.getColumn()`) from the `FrameFieldWriterException` anywhere, or use 
check for it's instanceof, then I suppose we can get rid of it. 



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org



[PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)

2024-03-14 Thread via GitHub


gargvishesh opened a new pull request, #16130:
URL: https://github.com/apache/druid/pull/16130

   Current Runtime Exceptions generated while writing frames only include the 
exception itself without including the name of the column they were encountered 
in, for e.g. 
   
   `java.lang.RuntimeException: java.lang.ClassCastException: class 
java.lang.Long cannot be cast to class java.lang.String (java.lang.Long and 
java.lang.String are in module java.base of loader 'bootstrap')`
   
   Start including the col name for easier debugging. 
   
   New Exception:
   java.lang.RuntimeException: 
org.apache.druid.frame.write.FrameFieldWriterException: Error [class 
java.lang.Long cannot be cast to class java.lang.String (java.lang.Long and 
java.lang.String are in module java.base of loader 'bootstrap')] while writing 
frame for column [arrayString]`
   


-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org