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

Vavr arguments do not work with SqlObjects #2529

Closed
diversit opened this issue Nov 6, 2023 · 3 comments · Fixed by #2548
Closed

Vavr arguments do not work with SqlObjects #2529

diversit opened this issue Nov 6, 2023 · 3 comments · Fixed by #2548
Assignees
Labels

Comments

@diversit
Copy link

diversit commented Nov 6, 2023

Using Vavr object like an Option as an argument with SqlObjects like

@SqlQuery("select * from something where :name is null or name = :name order by id")
@RegisterBeanMapper(Something.class)
List<Something> selectByOptionName(@Bind Option<String> name);

does not work.

Based on the TestVavrValueArgumentFactoryWithDB in JDBI Vavr module, I created these test cases for using an Option as an argument.

public class TestVavrValueArgumentFactoryWithDBSqlObjects {


    private static final Something ERICSOMETHING = new Something(1, "eric");
    private static final Something BRIANSOMETHING = new Something(2, "brian");

    @RegisterExtension
    public JdbiExtension h2Extension = JdbiExtension.h2().withInitializer(TestingInitializers.something()).installPlugins();

    @BeforeEach
    public void createTestData() {
        Handle handle = h2Extension.getSharedHandle();
        handle.createUpdate("insert into something (id, name) values (1, 'eric')").execute();
        handle.createUpdate("insert into something (id, name) values (2, 'brian')").execute();
    }


    @Test
    public void testGetOptionShouldReturnCorrectRowUsingExtension() {
        List<Something> result = h2Extension.getJdbi()
                .withExtension(Dao.class, dao -> dao.selectByOptionName(Option.of("eric")));

        assertThat(result).hasSize(1);
    }

    @Test
    public void testGetOptionEmptyShouldReturnAllRowsUsingExtension() {
        List<Something> result = h2Extension.getJdbi()
                .withExtension(Dao.class, dao -> dao.selectByOptionName(Option.none()));

        assertThat(result).hasSize(2);
    }

    interface Dao {

        @SqlQuery("select * from something where :name is null or name = :name order by id")
        @RegisterBeanMapper(Something.class)
        List<Something> selectByOptionName(@Bind Option<String> name);
    }

With a Some the query works, but with a None it does not.
This is somewhat different than what we see in our codebase. Then the test with a Some value also fails with

org.jdbi.v3.core.statement.UnableToExecuteStatementException: org.postgresql.util.PSQLException: ERROR: operator does not exist: integer = bigint[]
  Hint: No operator matches the given name and argument types. You might need to add explicit type casts.
  Position: 50 [statement:"SELECT * FROM our_table  WHERE id = :id ", arguments:{positional:{0:Some(47)}, named:{id:Some(47)}, finder:[]}]

While debugging I notice that the VavrValueArgumentFactory.build method does not get called when using SqlObject.
It does get called when using the Java Api .bind("name", Option.none()).

@hgschmie
Copy link
Contributor

Hi @diversit,

Thank you for opening a bug report with the Jdbi team. I can reproduce that problem locally (also the Postgres errors).

@hgschmie
Copy link
Contributor

The problem is that Option implements Iterable and when creating the arugment, the SqlArrayArgumentFactory "snatches" the argument because it deals with all iterable arguments. This is a shortcoming in factory selection. I'll figure something out.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Nov 25, 2023
Turns out, this has never worked before. To work with SqlObjects,
the argument factory needs to implement ArgumentFactory.Preparable,
otherwise another factory (in this case one that accepts anything
iterable) would take precedence.

Rewrite the factory to support Preparable, add unit tests suggested
by @diversit.

Fixes jdbi#2529
@hgschmie
Copy link
Contributor

Will be fixed in the next release (3.42.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants