kgyrtkirk commented on code in PR #15371:
URL: https://github.com/apache/druid/pull/15371#discussion_r1400604814
##########
processing/src/main/java/org/apache/druid/segment/ObjectColumnSelector.java:
##########
@@ -19,75 +19,50 @@
package org.apache.druid.segment;
+import org.apache.druid.error.DruidException;
+
/**
- * This class is convenient for implementation of "object-sourcing" {@link
ColumnValueSelector}s, it provides default
- * implementations for all {@link ColumnValueSelector}'s methods except {@link
#getObject()} and {@link
- * #classOfObject()}.
- *
- * This class should appear ONLY in "extends" clause or anonymous class
creation, but NOT in "user" code, where
- * {@link BaseObjectColumnValueSelector} must be used instead.
+ * Object based column selector.
*
- * This is a class rather than an interface unlike {@link LongColumnSelector},
{@link FloatColumnSelector} and {@link
- * DoubleColumnSelector} solely in order to make {@link #isNull()} method
final.
+ * This abstract class provides a {@link ColumnValueSelector} based on the
methods of {@link BaseObjectColumnValueSelector}.
*/
public abstract class ObjectColumnSelector<T> implements ColumnValueSelector<T>
{
- /**
- * @deprecated This method is marked as deprecated in ObjectColumnSelector
to minimize the probability of accidental
- * calling. "Polymorphism" of ObjectColumnSelector should be used only when
operating on {@link ColumnValueSelector}
- * objects.
- */
- @Deprecated
+ private static final String COLUMN_IS_NULL_ERROR_MESSAGE = "Invalid usage
pattern: method returning primitive called - but the column is null";
+
@Override
- public float getFloat()
+ public final float getFloat()
{
T value = getObject();
if (value == null) {
- return 0;
+ throw DruidException.defensive(COLUMN_IS_NULL_ERROR_MESSAGE);
}
return ((Number) value).floatValue();
}
- /**
- * @deprecated This method is marked as deprecated in ObjectColumnSelector
to minimize the probability of accidental
- * calling. "Polymorphism" of ObjectColumnSelector should be used only when
operating on {@link ColumnValueSelector}
- * objects.
- */
- @Deprecated
Review Comment:
I don't understand why it was marked like that
* the implemented interface methods can be deprecated - but they aren't
* it did provided a working implementation (no idea why...)
* it has the `@Deprecated` annotation for >5 years - should we really carry
it forward?
* is anyone committed to fix this deprecation?
* what are benefits of doing so?
* the apidoc's were a bit contradicting; but the `is convenient for
implementation of "object-sourcing" seemed the most accurate.
* the class itself is not `@Deprecated` so anyone can just extend it and use
it
because of the above I've just removed them...
I think instead of `@Deprecated`: if the implementation should reject calls
to `getLong` and friends it should just throw an `Exception` in the default
implementation explaining the situation - but even in that case it can't really
be deprecated as this class is not the one declaring those methods.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
