xiangfu0 commented on a change in pull request #7692:
URL: https://github.com/apache/pinot/pull/7692#discussion_r743176327
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV3.java
##########
@@ -175,8 +175,11 @@ public DataTableImplV3(ByteBuffer byteBuffer)
}
@Override
- public void addException(ProcessingException processingException) {
- _errCodeToExceptionMap.put(processingException.getErrorCode(),
processingException.getMessage());
+ public void addException(Exception exception) {
+ if (exception instanceof ProcessingException) {
Review comment:
any chance we add other exceptions here?
Shall we add a log here if the exception is not ProcessingException?
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/data/DataTable.java
##########
@@ -33,7 +32,7 @@
// TODO: remove this when we stop supporting DataTable V2.
String EXCEPTION_METADATA_KEY = "Exception";
- void addException(ProcessingException processingException);
+ void addException(Exception processingException);
Review comment:
Shall we consider make
```
public interface DataTable<E> {
...
void addException(E processingException);
```
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/data/DataTable.java
##########
@@ -33,7 +32,7 @@
// TODO: remove this when we stop supporting DataTable V2.
String EXCEPTION_METADATA_KEY = "Exception";
- void addException(ProcessingException processingException);
+ void addException(Exception processingException);
Review comment:
Shall we consider make
```
public interface DataTable<E> {
...
void addException(E processingException);
```
Then in
```
DataTableImplV3<ProcessingException>
...
public void addException(ProcessingException processingException) {
```
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV2.java
##########
@@ -148,8 +151,13 @@ public DataTableImplV2(ByteBuffer byteBuffer)
}
@Override
- public void addException(ProcessingException processingException) {
Review comment:
then should it goto BaseDataTable?
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV2.java
##########
@@ -148,8 +151,13 @@ public DataTableImplV2(ByteBuffer byteBuffer)
}
@Override
- public void addException(ProcessingException processingException) {
Review comment:
then should this goto BaseDataTable? same for V3
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV2.java
##########
@@ -43,6 +45,7 @@
// FIXED_SIZE_DATA (START|SIZE)
// VARIABLE_SIZE_DATA (START|SIZE)
private static final int HEADER_SIZE = Integer.BYTES * 13;
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DataTableImplV2.class);
Review comment:
this is not used
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableImplV3.java
##########
@@ -71,6 +73,8 @@
*/
public class DataTableImplV3 extends BaseDataTable {
private static final int HEADER_SIZE = Integer.BYTES * 13;
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DataTableImplV3.class);
Review comment:
not used.
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/BaseDataTable.java
##########
@@ -40,6 +40,9 @@
* Base implementation of the DataTable interface.
*/
public abstract class BaseDataTable implements DataTable {
+
+ public static final int RESERVED_DEFAULT_EXCEPTION_CODE = -1;
Review comment:
we have unknown exception code I think
--
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...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
