Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
/**
* 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.kafka.copycat.data;

import org.apache.kafka.copycat.data.Schema.Type;
import org.apache.kafka.copycat.errors.SchemaProjectorException;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* <p>
* SchemaProjector is utility to project a value between compatible schemas and throw exceptions
* when non compatible schemas are provided.
* </p>
*/

public class SchemaProjector {

/**
* @param writer the schema that is used to write the record

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fill in a regular javadoc description. It'll probably be similar to the one on the class.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* @param reader the schema that is used to read the record
* @param record the value to project from writer schema to reader schema
* @return the projected value with reader schema
* @throws SchemaProjectorException
*/
public static Object project(Schema writer, Schema reader, Object record) throws SchemaProjectorException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the terminology here. Reader and writer don't make sense in this context since nothing is being read or written. Maybe source and target?

Also, it's possible the intent might be clearer if writer and record were next to each other in the argument list since record should be in the writer format and being converted to reader format.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (writer.isOptional() && !reader.isOptional()) {
if (reader.defaultValue() != null) {
if (record != null) {
return projectRequiredSchema(writer, reader, record);
} else {
return reader.defaultValue();
}
} else {
throw new SchemaProjectorException("Writer schema is optional, however, reader schema does not provide a default value.");
}
} else {
if (record != null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the record is null, this isn't doing any schema compatibility checks besides the check for optional/required check at the top of this method. This is causing the logical type test to fail.

return projectRequiredSchema(writer, reader, record);
}
return null;
}
}

private static Object projectRequiredSchema(Schema writer, Schema reader, Object record) throws SchemaProjectorException {
Object result = null;
// handle logical types
if (reader.name() != null) {
return projectLogical(writer, reader, record);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need special handling for logical types? Shouldn't they just be like any other type where we check compatibility and otherwise the data just passes through?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
switch (reader.type()) {
case INT8:
case INT16:
case INT32:
case INT64:
case FLOAT32:
case FLOAT64:
case BOOLEAN:
case BYTES:
case STRING:
result = projectPrimitive(writer, reader, record);
break;
case STRUCT:
result = projectStruct(writer, reader, (Struct) record);
break;
case ARRAY:
result = projectArray(writer, reader, record);
break;
case MAP:
result = projectMap(writer, reader, record);
}
return result;
}

private static Object projectStruct(Schema writer, Schema reader, Struct writerStruct) throws SchemaProjectorException {
Struct readerStruct = new Struct(reader);
for (Field readerField : reader.fields()) {
String fieldName = readerField.name();
Field writerField = writer.field(fieldName);
if (writerField != null) {
Object writerFieldValue = writerStruct.get(fieldName);
Object readerFieldValue = project(writerField.schema(), readerField.schema(), writerFieldValue);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not critical, but you might want to catch any exceptions from this project call and wrap them in another SchemaProjectorException so you can include info about which field failed projection.

readerStruct.put(fieldName, readerFieldValue);
} else {
Object readerDefault;
if (readerField.schema().defaultValue() != null) {
readerDefault = readerField.schema().defaultValue();
} else {
throw new SchemaProjectorException("Cannot project " + writer.schema() + " to " + reader.schema());
}
readerStruct.put(fieldName, readerDefault);
}
}
return readerStruct;
}

private static Object projectArray(Schema writer, Schema reader, Object record) throws SchemaProjectorException {
if (writer.type() != reader.type()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is repeated in many of the methods, and I think there are a few other parts of the schema that you want to check are equal. Can we lift this up into projectRequiredSchema? I think you also want to check that the name, version, and parameters are equal. Then each of these projectX methods only needs to check any pieces that are specific to it, e.g. the key and value schemas.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

throw new SchemaProjectorException("Schema type mismatch. Writer type: " + writer.type() + " and Reader type: " + reader.type());
}
Type writerValueType = writer.valueSchema().type();
Type readerValueType = reader.valueSchema().type();
if (writerValueType != readerValueType && !promote(writerValueType, readerValueType)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think you want the same type of check here -- comparing the types is the first step, but you also need to check other parts of the schemas.

Alternatively, if you rely in the recursive call to project(writer.valueSchema(), reader.valueSchema(), entry) in the loop below, then wouldn't that already be doing the check for matching types such that you could get rid of this check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

throw new SchemaProjectorException("Value schema type mismatch for array. Writer value schema type: " + writer.type() + " and Reader value schema type: " + reader.type());
}
List<?> array = (List<?>) record;
List<Object> retArray = new ArrayList<>();
for (Object entry : array) {
retArray.add(project(writer.valueSchema(), reader.valueSchema(), entry));
}
return retArray;
}

private static Object projectMap(Schema writer, Schema reader, Object record) throws SchemaProjectorException {
if (writer.type() != reader.type()) {
throw new SchemaProjectorException("Schema type mismatch. Writer type: " + writer.type() + " and Reader type: " + reader.type());
}
Type writerKeyType = writer.keySchema().type();
Type readerKeyType = reader.keySchema().type();
if (writerKeyType != readerKeyType && !promote(writerKeyType, readerKeyType)) {
throw new SchemaProjectorException("Key schema type mismatch for array. Writer key schema type: " + writer.type() + " and Reader key schema type: " + reader.type());
}
Type writerValueType = writer.valueSchema().type();
Type readerValueType = reader.valueSchema().type();
if (writerValueType != readerValueType && !promote(writerValueType, readerValueType)) {
throw new SchemaProjectorException("Value schema type mismatch for array. Writer value schema type: " + writer.type() + " and Reader value schema type: " + reader.type());
}
Map<?, ?> map = (Map<?, ?>) record;
Map<Object, Object> retMap = new HashMap<>();
for (Map.Entry<?, ?> entry : map.entrySet()) {
Object key = entry.getKey();
Object value = entry.getValue();
Object retKey = project(writer.keySchema(), reader.keySchema(), key);
Object retValue = project(writer.valueSchema(), writer.valueSchema(), value);
retMap.put(retKey, retValue);
}
return retMap;
}

private static Object projectLogical(Schema writer, Schema reader, Object record) throws SchemaProjectorException {
if (writer.name() == null || !writer.name().equals(reader.name())) {
throw new SchemaProjectorException("Schema does not match for " + writer.name() + " and " + reader.name());
}

switch (reader.name()) {
case Decimal.LOGICAL_NAME:
case Date.LOGICAL_NAME:
case Time.LOGICAL_NAME:
case Timestamp.LOGICAL_NAME:
return record;
default:
throw new SchemaProjectorException("Not a valid logical type: " + reader.name());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just want to get rid of the special handling for logical types, but if it stays, the handling here needs to change. Any user defined schema can have a name too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
}

private static Object projectPrimitive(Schema writer, Schema reader, Object record) throws SchemaProjectorException {
if (writer.type() != reader.type() && !promote(writer.type(), reader.type())) {
throw new SchemaProjectorException("Schema type mismatch. Writer type: " + writer.type() + " and Reader type: " + reader.type());
}
if (writer.isOptional() && !reader.isOptional()) {
if (reader.defaultValue() != null) {
if (record != null) {
return record;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about promotion? We need to actually perform the type promotion. It isn't valid, for example, to use an int8 field's Byte value directly for a float32 field.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

} else {
return reader.defaultValue();
}
} else {
throw new SchemaProjectorException("Writer schema is optional, however, reader schema does not provide a default value.");
}
} else {
return record;
}
}

private static boolean promote(Type writerType, Type readerType) {
Type[] numericTypes = {Type.INT8, Type.INT16, Type.INT32, Type.INT64, Type.FLOAT32, Type.FLOAT64};
int index = -1;
for (int i = 0; i < numericTypes.length; ++i) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list is small anyway, so I'm not too worried about the cost, but this seems wasteful. Seems like this promotion check should be a simple lookup in a precomputed table.

if (writerType.equals(numericTypes[i])) {
index = i;
break;
}
}
if (index == -1) {
return false;
}
boolean result = false;
for (int i = index; i < numericTypes.length; ++i) {
result = result || readerType.equals(numericTypes[i]);
}
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* 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.kafka.copycat.errors;

public class SchemaProjectorException extends DataException {
public SchemaProjectorException(String s) {
super(s);
}

public SchemaProjectorException(String s, Throwable throwable) {
super(s, throwable);
}

public SchemaProjectorException(Throwable throwable) {
super(throwable);
}
}
Loading