[FLINK-2678]DataSet API does not support multi-dimensional arrays as keys #1566
Conversation
|
@StephanEwen could you also help reviewing the code? |
| import java.lang.reflect.Array; | ||
| import java.util.Arrays; | ||
|
|
||
| public class GenericArrayComparator<T> extends TypeComparator<T[]> implements java.io.Serializable { |
There was a problem hiding this comment.
Why not calling it ObjectArrayComparator?
|
Thanks for your contribution @sbcd90. I think the PR needs still a bit of work before it can be merged. The implementation of |
|
@tillrohrmann Thanks for the review. I had a few questions based on your comments. Kindly help me in getting the questions answered so that I can proceed. |
|
Hello @tillrohrmann ..I have made all the changes you have mentioned.
Please review my code now. Kindly let me know if there are any further changes required. Kindly let me know if this can be merged now also. |
|
Hello @tillrohrmann please review the commit as the work to be done is nearly complete I believe. Please comment. |
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private TypeComparator<? super Object> getBaseComparatorInfo(TypeInformation<? extends Object> componentInfo, boolean sortOrderAscending, ExecutionConfig executionConfig) { |
There was a problem hiding this comment.
Why are you extracting for certain types the element comparator (for example the ObjectArrayTypeInfo) and for other you create the TypeComparator (for example the CompositeTypeInfo)? I don't get it. Why do you need the getBaseComparatorInfo method at all? Simply check in createComparator the different subtypes and then create the TypeComparator.
|
I think the PR is still not in a good shape. I pointed out the problematic code sections. Also please bear in mind that people will always try to review your code as soon as possible. But sometimes people are just busy and it takes some time for them to review a PR. In such a case, one simply has to be a little bit patient with them. |
Hello,
@tillrohrmann I have added support for multi-dimensional arrays as keys in Dataset api. Please review & merge.