fdb-record-layer icon indicating copy to clipboard operation
fdb-record-layer copied to clipboard

`Type.Record.Field`'s protobuf index is used to lookup field descriptor

Open hatyo opened this issue 1 year ago • 0 comments

We normally rely on field ordinals to lookup a Field of a Record or set its value in the Record's corresponding dynamic message descriptor such as here:

    @Nullable
    @Override
    public <M extends Message> Object eval(@Nonnull final FDBRecordStoreBase<M> store, @Nonnull final EvaluationContext context) {
        final var typeRepository = context.getTypeRepository();
        final var resultMessageBuilder = newMessageBuilderForType(typeRepository);
        final var descriptorForType = resultMessageBuilder.getDescriptorForType();
        final var fieldDescriptors = descriptorForType.getFields();

        final var fields = Objects.requireNonNull(getResultType().getFields());
        var i = 0;
        for (final var child : getChildren()) {
            final var field = fields.get(i);
            final var fieldType = field.getFieldType();
            var childResult = deepCopyIfNeeded(typeRepository, fieldType, child.eval(store, context));
            if (childResult != null) {
                final var fieldDescriptor = fieldDescriptors.get(i);
                if (fieldType.isArray() && fieldType.isNullable()) {
                    final var wrappedDescriptor = fieldDescriptor.getMessageType();
                    final var wrapperBuilder = DynamicMessage.newBuilder(wrappedDescriptor);
                    wrapperBuilder.setField(wrappedDescriptor.findFieldByName(NullableArrayTypeUtils.getRepeatedFieldName()), childResult);
                    childResult = wrapperBuilder.build();
                }
                resultMessageBuilder.setField(fieldDescriptor, childResult);
            } else {
                Verify.verify(fieldType.isNullable());
            }
            i++;
        }
        return resultMessageBuilder.build();
    }

However, in Accumulator#finish() we directly use the proto index:

@Nonnull
            @Override
            public Object finish() {
                final var resultMessageBuilder = newMessageBuilderForType(typeRepository);
                final var descriptorForType = resultMessageBuilder.getDescriptorForType();

                var i = 0;
                final var fields = Objects.requireNonNull(getResultType().getFields());

                for (final var childAccumulator : childAccumulators) {
                    final var finalResult = childAccumulator.finish();
                    if (finalResult != null) {
                        resultMessageBuilder.setField(descriptorForType.findFieldByNumber(fields.get(i).getFieldIndex()), finalResult);
                    }
                    i ++;
                }

                return resultMessageBuilder.build();
            }

We should probably make more consistent use of field ordinals.

hatyo avatar Oct 27 '23 10:10 hatyo