Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for RowN.mapping(Function<? super Object[], ? extends U>) #12515

Closed
alf opened this issue Oct 13, 2021 · 6 comments
Closed

Support for RowN.mapping(Function<? super Object[], ? extends U>) #12515

alf opened this issue Oct 13, 2021 · 6 comments

Comments

@alf
Copy link

alf commented Oct 13, 2021

Use case:

We have situations where we work on groups of fields where the group is of unknown size. The current use case is composite keys, but I can see this as useful for other groups as well.

By wrapping the fields in a row we can attach a mapping function to the resulting selectfield at query build time, something that is very powerful when building dynamic reusable queries.

Unfortunately the RowN interface does not provide the mapping method, thus we have to switch over the group size and create a lot of boilerplate/duplicate code.

Possible solution you'd like to see:

Being able to attach a mapping on a RowN. This mapping could provide us with a Record that can be used together with the original group of fields to get the values and instantiate the user class we want.

The parameters provided to the mapping-method could include the group of fields as well as the record, but this isn’t strictly necessary.

Possible workarounds:

Switch over the group size and create the specific methods needed.

@lukaseder
Copy link
Member

Thanks for your suggestion. I don't think it's possible to abstract over arity in Java, other than what Scala / Haskell did with HList, or what TypeScript does with their turing complete recursive generics, but those things are practical only with built-in language support.

RowN is a row type of degree > 22. It's not compatible with e.g. Row2. Did you mean to add mapping methods on Row, instead? But what would such a method look like? Would it take a Function1<? super Object[], ? extends R> argument? That would conflict with Row1.mapping(Function1), incompatibly, due to erasure.

Also, from my experience, it's generally not a good idea to overload methods taking lambda arguments, because method references (and in this case, constructor references) suddenly stop working, which would be a pain if the arity is high. A new method name could be invented, but this kinda hints at the idea or approach not being clean.

Switch over the group size and create the specific methods needed.

If you look at jOOQ's internals, that's precisely what jOOQ does all the time. Look e.g. at Tools.row0(FieldsImpl), Tools.recordType(int), Tools.recordFactory(Class, AbstractRow). There can be more such tools internally, or also as public API.

If you could show a specific example of what you're trying to do, there might be a better API possible than the one you asked for. For example, there's a possibility of embedded keys being the answer to your question about composite keys:
https://www.jooq.org/doc/latest/manual/code-generation/codegen-embeddable-types/codegen-embedded-keys/

But I'm just guessing. A real world example of what actual problem you're trying to solve would be helpful.

@alf
Copy link
Author

alf commented Oct 13, 2021

I understand the issues with arity and suspected that this feature might be wishful thinking. I figured raising it might be worth while even so. I'm not too unhappy with the switch-based approach, it just feels a bit off. :)

I looked at using embedded keys, but ran into issues since we make extensive use of natural keys and need to access the columns individually as well. I found no way of doing that. Didn't think of raising this as an issue to be honest.

Here's a snippet of what I'm currently doing:

public static SelectField<String> getId(Table<?> table, Key<?> key) {
        var fs = keyFields(table, key);

        switch (fs.size()) {
            case 1:
                return createRow(fs.get(0));
            case 2:
                return createRow(fs.get(0), fs.get(1));
            case 3:
                return createRow(fs.get(0), fs.get(1), fs.get(2));
            case 4:
                return createRow(fs.get(0), fs.get(1), fs.get(2), fs.get(3));
            case 5:
                return createRow(fs.get(0), fs.get(1), fs.get(2), fs.get(3), fs.get(4));
            default:
                throw new IllegalArgumentException("getId only supports keys with 5 columns, we must add more");
        }
    }

    private static <T1, T2, T3, T4, T5> SelectField<String> createRow(Field<T1> f1, Field<T2> f2, Field<T3> f3, Field<T4> f4, Field<T5> f5) {
        return row(f1, f2, f3, f4, f5).mapping(String.class, (v1, v2, v3, v4, v5) -> csv(toStr(f1, v1), toStr(f2, v2), toStr(f3, v3), toStr(f4, v4), toStr(f5, v5)));
    }

[snip]

@lukaseder
Copy link
Member

I understand the issues with arity and suspected that this feature might be wishful thinking.

Well, we can hope! I, for one, am hoping for higher kinded generics, or even what TypeScript has. Though, that too, is probably wishful thinking, in Java

it just feels a bit off. :)

It does. But so does stopping type safety at degree 22... 😅. Within jOOQ, we have an API and implementation code generator. Specifically, we use Xtend's templates to generate the code. It gets too boring to write, otherwise. Once that's in place, it's not as bad anymore to support degrees 1 - 22 for each such case.

I looked at using embedded keys, but ran into issues since we make extensive use of natural keys and need to access the columns individually as well.

Ah yes, embeddable keys replace their columns by default, though it's possible to turn that off (though, not for keys yet):
https://www.jooq.org/doc/latest/manual/code-generation/codegen-embeddable-types/codegen-embeddable-types-replacing-fields

There are some issues to fix this, already

Here's a snippet of what I'm currently doing:

I see, thanks for the example. Well, as a workaround, you could use DSL.rowField(RowN) here, and instead of calling the Row[N].mapping() convenience method, you'd call the underlying Field.convertFrom() method directly.

DSL::rowField is deprecated, because most use-cases can be implemented using the new Row[N] <: SelectField<Record[N]> subtype hierarchy, and I'm not too keen to undeprecate it. The Field representation (instead of SelectField) caused quite a few open questions when rowField() was still experimental.

However, I get your point now. I'm not sure if RowN::mapping can be a thing, because it feels wrong to pass an Object[] to the argument function. But given that mapping is just convenience for convertFrom(), perhaps we can pull up convertXYZ() from Field to SelectField, though. That would improve your getId() method, removing the need to switch over arity.

I was planning to review this as part of #7100 (comment), and then delayed it until the implementation of #4727, which raises a few questions in terms of type hierarchy consistency.

Will review again for 3.16

@alf
Copy link
Author

alf commented Oct 14, 2021

Thank you for taking the time to understand my need. We’ll get by with my current implementation for now, and then move to .convertFrom or embedded keys in the future. I agree that it’s better to leave RowField deprecated.

@lukaseder lukaseder added this to To do in 3.14 XML and JSON via automation Feb 16, 2022
@lukaseder lukaseder changed the title Support for RowN.mapping Support for RowN.mapping(Function<? super Object[], ? extends U>) Feb 16, 2022
@lukaseder
Copy link
Member

I just needed this myself, recently, and was probably as surprised as you were, when it wasn't there. The most obvious choice of method signature would be:

<U> SelectField<U> mapping(Function<? super Object[], ? extends U> function);
<U> SelectField<U> mapping(Class<U> uType, Function<? super Object[], ? extends U> function);

An alternative would be to apply Collection<?> to the argument function, as we always overload T|Collection<T> throughout the API, including the DSL::row constructors. I have a feeling that Object[] is more consistent with the rest of jOOQ, even if it's a bit less convenient.

@lukaseder
Copy link
Member

Implemented in jOOQ 3.17.0. Thanks again for your feature request.

3.14 XML and JSON automation moved this from To do to Done Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants