You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/11/26 14:26:55 UTC
[GitHub] [ignite-3] AMashenkov opened a new pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
AMashenkov opened a new pull request #474:
URL: https://github.com/apache/ignite-3/pull/474
https://issues.apache.org/jira/browse/IGNITE-15907
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #474:
URL: https://github.com/apache/ignite-3/pull/474#discussion_r766526287
##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/marshaller/reflection/Marshaller.java
##########
@@ -141,94 +133,112 @@ public static Marshaller createMarshaller(Columns cols, Class<? extends Object>
*
* @param obj Object.
* @param writer Row writer.
- * @throws MarshallerException If failed.
+ * @throws MarshallerException If failed to marshall given object to a row.
*/
public abstract void writeObject(Object obj, RowAssembler writer) throws MarshallerException;
/**
- * Marshaller for objects of natively supported types.
+ * Marshaller for key/value objects of natively supported types. The case when a whole object maps to a single column.
*/
static class SimpleMarshaller extends Marshaller {
- /** Identity accessor. */
- private final FieldAccessor fieldAccessor;
+ /** Individual column binding. */
+ private final ColumnBinding columnBinding;
/**
* Creates a marshaller for objects of natively supported type.
*
- * @param fieldAccessor Identity field accessor for objects of natively supported type.
+ * @param columnBinding Identity field binding for objects of natively supported type.
*/
- SimpleMarshaller(FieldAccessor fieldAccessor) {
- this.fieldAccessor = fieldAccessor;
+ SimpleMarshaller(ColumnBinding columnBinding) {
+ this.columnBinding = columnBinding;
}
/** {@inheritDoc} */
@Override
- public @Nullable
- Object value(Object obj, int fldIdx) {
+ public @Nullable Object value(Object obj, int fldIdx) throws MarshallerException {
assert fldIdx == 0;
- return fieldAccessor.value(obj);
+ return columnBinding.value(obj);
}
/** {@inheritDoc} */
@Override
- public Object readObject(Row reader) {
- return fieldAccessor.read(reader);
+ public Object readObject(Row reader) throws MarshallerException {
+ try {
+ return columnBinding.columnValue(reader);
+ } catch (Throwable e) {
+ throw new MarshallerException("Failed to read column: colIdx" + columnBinding.colIdx, e);
+ }
}
/** {@inheritDoc} */
@Override
public void writeObject(Object obj, RowAssembler writer) throws MarshallerException {
- fieldAccessor.write(writer, obj);
+ try {
+ columnBinding.write(writer, obj);
+ } catch (Throwable e) {
+ throw new MarshallerException("Failed to write column: colIdx" + columnBinding.colIdx, e);
+ }
}
}
/**
- * Marshaller for POJOs.
+ * Marshaller for POJOs/ The case when an object fields map to the columns.
Review comment:
```suggestion
* Marshaller for POJOs. The case when an object fields map to the columns.
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #474:
URL: https://github.com/apache/ignite-3/pull/474#discussion_r766489896
##########
File path: modules/api/src/main/java/org/apache/ignite/table/mapper/Mapper.java
##########
@@ -17,60 +17,175 @@
package org.apache.ignite.table.mapper;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import java.lang.reflect.Modifier;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.util.BitSet;
+import java.util.UUID;
/**
* Mapper interface defines methods that are required for a marshaller to map class field names to table columns.
*
- * @param <T> Mapped type.
+ * <p>NB: Anonymous, local, inner classes, and special types are NOT supported, and mapper will not be created for them: e.g. interfaces,
+ * annotation, and so on.
+ *
+ * @param <T> Type of which objects the mapper handles.
+ * @apiNote Implementation shouldn't use this interface directly, please, use {@link PojoMapper} or {@link OneColumnMapper} instead.
+ * @see PojoMapper
+ * @see OneColumnMapper
*/
public interface Mapper<T> {
/**
- * Creates a mapper for a class.
+ * Shortcut method creates a mapper for class. Natively supported types will be mapped to a single schema column, otherwise individual
+ * object fields will be mapped to columns with the same name.
+ *
+ * <p>Note: Natively supported types can be mapped to a single key/value column, otherwise table operation will ends up with exception.
+ * Use {@link #of(Class, String)} instead, to map to a concrete column name.
*
- * @param cls Key class.
- * @param <K> Key type.
- * @return Mapper.
+ * @param cls Target type.
+ * @return Mapper for key objects representing a single key column.
+ * @throws IllegalArgumentException If {@code cls} is of unsupported kind.
Review comment:
two suggestions from my side:
1) Maybe parameter T will be more understandable than O.
2) maybe add @param for target type?
##########
File path: modules/api/src/main/java/org/apache/ignite/table/mapper/Mapper.java
##########
@@ -17,60 +17,175 @@
package org.apache.ignite.table.mapper;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import java.lang.reflect.Modifier;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.util.BitSet;
+import java.util.UUID;
/**
* Mapper interface defines methods that are required for a marshaller to map class field names to table columns.
*
- * @param <T> Mapped type.
+ * <p>NB: Anonymous, local, inner classes, and special types are NOT supported, and mapper will not be created for them: e.g. interfaces,
+ * annotation, and so on.
+ *
+ * @param <T> Type of which objects the mapper handles.
+ * @apiNote Implementation shouldn't use this interface directly, please, use {@link PojoMapper} or {@link OneColumnMapper} instead.
+ * @see PojoMapper
+ * @see OneColumnMapper
*/
public interface Mapper<T> {
/**
- * Creates a mapper for a class.
+ * Shortcut method creates a mapper for class. Natively supported types will be mapped to a single schema column, otherwise individual
+ * object fields will be mapped to columns with the same name.
+ *
+ * <p>Note: Natively supported types can be mapped to a single key/value column, otherwise table operation will ends up with exception.
+ * Use {@link #of(Class, String)} instead, to map to a concrete column name.
*
- * @param cls Key class.
- * @param <K> Key type.
- * @return Mapper.
+ * @param cls Target type.
+ * @return Mapper for key objects representing a single key column.
+ * @throws IllegalArgumentException If {@code cls} is of unsupported kind.
Review comment:
Sorry, T already used here.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov merged pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
Posted by GitBox <gi...@apache.org>.
AMashenkov merged pull request #474:
URL: https://github.com/apache/ignite-3/pull/474
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #474:
URL: https://github.com/apache/ignite-3/pull/474#discussion_r766490856
##########
File path: modules/api/src/main/java/org/apache/ignite/table/mapper/Mapper.java
##########
@@ -17,60 +17,175 @@
package org.apache.ignite.table.mapper;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import java.lang.reflect.Modifier;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.util.BitSet;
+import java.util.UUID;
/**
* Mapper interface defines methods that are required for a marshaller to map class field names to table columns.
*
- * @param <T> Mapped type.
+ * <p>NB: Anonymous, local, inner classes, and special types are NOT supported, and mapper will not be created for them: e.g. interfaces,
+ * annotation, and so on.
+ *
+ * @param <T> Type of which objects the mapper handles.
+ * @apiNote Implementation shouldn't use this interface directly, please, use {@link PojoMapper} or {@link OneColumnMapper} instead.
+ * @see PojoMapper
+ * @see OneColumnMapper
*/
public interface Mapper<T> {
/**
- * Creates a mapper for a class.
+ * Shortcut method creates a mapper for class. Natively supported types will be mapped to a single schema column, otherwise individual
+ * object fields will be mapped to columns with the same name.
+ *
+ * <p>Note: Natively supported types can be mapped to a single key/value column, otherwise table operation will ends up with exception.
+ * Use {@link #of(Class, String)} instead, to map to a concrete column name.
*
- * @param cls Key class.
- * @param <K> Key type.
- * @return Mapper.
+ * @param cls Target type.
+ * @return Mapper for key objects representing a single key column.
+ * @throws IllegalArgumentException If {@code cls} is of unsupported kind.
Review comment:
Sorry, T already used here.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #474: IGNITE-15907: Fix generated marshaller and drop dead code.
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #474:
URL: https://github.com/apache/ignite-3/pull/474#discussion_r766489896
##########
File path: modules/api/src/main/java/org/apache/ignite/table/mapper/Mapper.java
##########
@@ -17,60 +17,175 @@
package org.apache.ignite.table.mapper;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import java.lang.reflect.Modifier;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.util.BitSet;
+import java.util.UUID;
/**
* Mapper interface defines methods that are required for a marshaller to map class field names to table columns.
*
- * @param <T> Mapped type.
+ * <p>NB: Anonymous, local, inner classes, and special types are NOT supported, and mapper will not be created for them: e.g. interfaces,
+ * annotation, and so on.
+ *
+ * @param <T> Type of which objects the mapper handles.
+ * @apiNote Implementation shouldn't use this interface directly, please, use {@link PojoMapper} or {@link OneColumnMapper} instead.
+ * @see PojoMapper
+ * @see OneColumnMapper
*/
public interface Mapper<T> {
/**
- * Creates a mapper for a class.
+ * Shortcut method creates a mapper for class. Natively supported types will be mapped to a single schema column, otherwise individual
+ * object fields will be mapped to columns with the same name.
+ *
+ * <p>Note: Natively supported types can be mapped to a single key/value column, otherwise table operation will ends up with exception.
+ * Use {@link #of(Class, String)} instead, to map to a concrete column name.
*
- * @param cls Key class.
- * @param <K> Key type.
- * @return Mapper.
+ * @param cls Target type.
+ * @return Mapper for key objects representing a single key column.
+ * @throws IllegalArgumentException If {@code cls} is of unsupported kind.
Review comment:
two suggestions from my side:
1) Maybe parameter T will be more understandable than O.
2) maybe add @param for target 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org