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

Jaybird 5 trims returned data #729

Closed
EPluribusUnum opened this issue Jan 9, 2023 · 15 comments
Closed

Jaybird 5 trims returned data #729

EPluribusUnum opened this issue Jan 9, 2023 · 15 comments

Comments

@EPluribusUnum
Copy link

Hi Mark!

org.firebirdsql.jdbc.FBResultSet has hard coded setTrimTrailing(true) call.
Prior Jaybird versions returned the string without trim. This is a breaking change, Jaybird should/must return original data returned by the Firebird engine, we need the whitespace.
Please add an option to turn off this behaviour.

Thank you!

@mrotteveel
Copy link
Member

mrotteveel commented Jan 9, 2023

Jaybird doesn't hardcode the call trimTrailing(true), it depends on the parameter trimStrings of the prepareVars method. Jaybird 5 should trim under the same conditions as Jaybird 4.

I tried this with the following test, and the value is not trimmed as expected:

    @Test
    void testNoTrimByDefault() throws Exception {
        try (Connection connection = getConnectionViaDriverManager();
             Statement stmt = connection.createStatement();
             ResultSet rs = stmt.executeQuery("select 'abc   ' from rdb$database")) {
            assertTrue(rs.next(), "expected a row");
            assertEquals("abc   ", rs.getString(1), "value was unexpectedly trimmed");
        }
    }

Please provide a reproduction program.

@mrotteveel
Copy link
Member

Is this by any chance with CallableStatement#getString?

@EPluribusUnum
Copy link
Author

Yes, I tried with a procedure call

@mrotteveel
Copy link
Member

It looks like in FBCallableStatement it has a code path that does trim strings if you access through getString of the callable statement instead of a result set. I'll need to check the history to see why.

@mrotteveel
Copy link
Member

OK, I can reproduce it with:

   @Test
    void callableStatementShouldNotTrim() throws Exception{
        executeDDL(con, """
                create procedure char_return returns (val char(5)) as
                begin
                  val = 'A';
                end""");

        try (CallableStatement cstmt = con.prepareCall("execute procedure char_return")) {
            cstmt.execute();
            assertEquals("A    ", cstmt.getString(1));
        }
    }

@mrotteveel
Copy link
Member

This trim behaviour was also present in Jaybird 4 (and in Jaybird 3). If you experience different behaviour compared to Jaybird 4, I would really like to see a reproducible test case.

I guess you might be using getObject(..) instead of getString(..), as previous versions only applied the trim behaviour in getString(..), but I would like to see that confirmed.

@mrotteveel
Copy link
Member

Behaviour was introduced by accident in #167 (but only in 3.0, not in 2.2.6)

@EPluribusUnum
Copy link
Author

4.0.7.java8 (no trimming)

DefaultDatatypeCoder.decodeString(byte[]) line: 235	
FBWorkaroundStringField(FBStringField).getString() line: 163	
FBWorkaroundStringField.getString() line: 111	
FBWorkaroundStringField(FBField).getObject() line: 346	
FBResultSet(AbstractResultSet).getObject(int) line: 509	
FBCallableStatement(AbstractCallableStatement).getObject(int) line: 462	
DelegatingCallableStatement.getObject(int) line: 463	
DelegatingCallableStatement.getObject(int) line: 463	
ConfigurableConnection$ConfigurableCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
SQLCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
ProcedureCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
DBUtils.callableStatementToMap(CallableStatement, String) line: 1110	
DBUtils.executeProcIntoMap(Connection, String, Map<String,Object>) line: 3080	
TestManager.runSQL(SQL, List<Param>, Connection, boolean) line: 1066	
TestManager.access$2(TestManager, SQL, List, Connection, boolean) line: 982	
TestManager$TestRunner.check() line: 283	
TestManager$TestRunner(AbstractTestManager$AbstractTestRunner).run() line: 886	
AbstractTestManager$TestExecuter.call() line: 1050	
AbstractTestManager$TestExecuter.call() line: 1013	
FutureTask<V>.run() line: 266	
ThreadPoolExecutor.runWorker(ThreadPoolExecutor$Worker) line: 1149	
ThreadPoolExecutor$Worker.run() line: 624	
Thread.run() line: 750	

FBWorkaroundStringField.getString() line: 111; Here trimString = false

5.0.0.java8-beta-1 (trimming)

FBStringField.applyTrimTrailing(String) line: 214	
FBStringField.getString() line: 187	
FBStringField.getObject() line: 79	
FBResultSet.getObject(int) line: 518	
FBCallableStatement.getObject(int) line: 511	
DelegatingCallableStatement.getObject(int) line: 463	
DelegatingCallableStatement.getObject(int) line: 463	
ConfigurableConnection$ConfigurableCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
SQLCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
ProcedureCallableStatement(AbstractCallableStatementWrapper).getObject(int) line: 1029	
DBUtils.callableStatementToMap(CallableStatement, String) line: 1110	
DBUtils.executeProcIntoMap(Connection, String, Map<String,Object>) line: 3080	
TestManager.runSQL(SQL, List<Param>, Connection, boolean) line: 1066	
TestManager.access$2(TestManager, SQL, List, Connection, boolean) line: 982	
TestManager$TestRunner.check() line: 283	
TestManager$TestRunner(AbstractTestManager$AbstractTestRunner).run() line: 886	
AbstractTestManager$TestExecuter.call() line: 1050	
AbstractTestManager$TestExecuter.call() line: 1	
FutureTask<V>.run() line: 266	
ThreadPoolExecutor.runWorker(ThreadPoolExecutor$Worker) line: 1149	
ThreadPoolExecutor$Worker.run() line: 624	
Thread.run() line: 750	

FBStringField.applyTrimTrailing(String) line: 214; Here trimTrailing = true

@mrotteveel
Copy link
Member

mrotteveel commented Jan 10, 2023

Looking at your that stacktrace, your code is using getObject(..), which in Jaybird 5 does the exact same thing as getString(..), while in previous versions only FBResultSet.getString(..) would trim in this case, but FBResultSet.getObject(..) would not. If you run your code against Jaybird 4.0.7.java8 using getString instead of getObject, you will find that it did trim in that version as well.

To be clear, I acknowledge this is a bug (introduced in Jaybird 3, but now more visible because it is now also exposed through getObject and not only getString), but a reproducible test case to confirm I'm actually fixing the bug you're experiencing, and not a similar adjacent bug would be very helpful.

@mrotteveel
Copy link
Member

In Jaybird 4 the trim happens in AbstractResultSet.getString(int) based on the value of trimStrings of the result set, the trimString in FBWorkaroundStringField is a red herring, because it was never used.

@mrotteveel
Copy link
Member

mrotteveel commented Jan 10, 2023

    @Test
    public void callableStatementShouldNotTrim_729() throws Exception {
        executeDDL(con,
                "create procedure char_return returns (val char(5)) as\n" +
                "begin\n" +
                "  val = 'A';\n" +
                "end");

        try (CallableStatement cstmt = con.prepareCall("execute procedure char_return")) {
            cstmt.execute();
            ResultSet rs = cstmt.getResultSet();
            assertTrue("Expected a row", rs.next());
            assertEquals("A    ", rs.getString(1));
            assertEquals("A    ", cstmt.getObject(1));
            assertEquals("A    ", cstmt.getString(1));
        }
    }

In Jaybird 4, this test fails on cstmt.getString, in Jaybird 5 it already fails on cstmt.getObject.

Is this comparable to your usage?

@EPluribusUnum
Copy link
Author

		conn.createStatement().execute(
			"create or alter procedure char_return returns (val varchar(5)) as\n" + //varchar return
				"begin\n" +
				"  val = 'A ';\n" + //note the trailing space
				"end");
		conn.commit();
		CallableStatement cs = conn.prepareCall("execute procedure char_return(?)");
		cs.registerOutParameter(1, Types.VARCHAR);
		cs.execute();
		assertEquals("A ", cs.getObject(1).toString());

This passes on 4.0.7 and fails on 5.0.0

@mrotteveel
Copy link
Member

Yes, but if you use cs.getString(1) it will also fail on Jaybird 4.0.7, right?

@EPluribusUnum
Copy link
Author

Yes, with getString(1) it fails on 4.0.7 too.

@mrotteveel
Copy link
Member

Fix can be tested with jaybird-5.0.1.java11-SNAPSHOT (and jaybird-5.0.1.java8-SNAPSHOT) from https://oss.sonatype.org/content/repositories/snapshots

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