Add another flexible way for JDBC paging (manual mode)

[dingx1]

Release notes

Added jdbc_paging_mode option to choose if use explicit pagination in statements and avoid the initial count query or use auto to delegate to the underlying library.

What does this PR do?

This commit introduces a new configuration option to avoid the initial count select statement executed by Sequel in case of paginated queries.
The paginated queries could be done now with initial count or not, in case jdbc_paging_mode is values explicit the plugin executes paged queries till it reach a page with less rows than the expected, instead of relying on the total row count.
In this case the SQL statement has to explicitly use the pagination keywords (like LIMIT and OFFSET) receiving as :offset and :size as implicit parameters.
When the paging mode jdbc_paging_mode is set to auto then the pagination happens automatically without the intervention of the user to create paginated query.

Why is it important/What is the impact to the user?

In some circumstances, like stored procedure call with pagination bounds, the initial count query is simply not meaningful.
In cases where the pages to retrieve are few the initial count query could be an useful overhead.
In some complex nested queries the count could generate extra work that's not usefull

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files (and/or docker env variables)
• I have added tests that prove my fix is effective or that my feature works

Author's Checklist

• check it with a local Logstash test.

How to test this PR locally

• spin up a local DB
• configure a JDBC input query with pagination and with the new jdbc_paging_avoid_count and check it retrieves all the expected rows. In logs should be present a line for each paged query

input {
  jdbc {
    jdbc_driver_library => "/path/mysql-connector-java-8.0.26.jar"
    jdbc_driver_class => "Java::com.mysql.cj.jdbc.Driver"
    jdbc_connection_string => "jdbc:mysql://localhost:3306/test_logstash"

    jdbc_user => "user"
    jdbc_password => "s3cret"

    jdbc_default_timezone => "UTC"

    schedule => "* * * * *"

    use_column_value => true
    tracking_column => "log_id"
    tracking_column_type => "numeric"
    last_run_metadata_path => ".last_run"
    
    statement => "SELECT * FROM data_log WHERE log_id > :sql_last_value LIMIT :size OFFSET :offset"
    jdbc_paging_enabled => true
    jdbc_page_size => 7
    jdbc_paging_avoid_count => true
  }
}

output {
  stdout {
    codec => "rubydebug"
  }
}

Related issues

• Superseeds Add another flexible way for JDBC paging #90

Use cases

Simple query with small result sets

In a simple query like:

SELECT * from test_table

the automatic pagination would generate the count query plus the paginated queries.
With small resultsets it issues 2 queries instead of one query.

A stored procedure that requires pagination information. Suppose you have a stored like

CALL fetch_my_data(:sql_last_value, <offset>, <size>)

the automatic pagination would generate an invalid SQL statement:

SELECT count(*) AS count FROM ( CALL fetch_my_data(:sql_last_value) ) AS t1 LIMIT 1
SELECT * FROM ( CALL fetch_my_data(:sql_last_value) ) AS t1 LIMIT 10 OFFSET 0
.
.

which is not what's expected.

Nested queries with pagination on the inner query

Suppose the query you want to paginate is the inner on, like in:

SELECT * FROM test_table2 WHERE id IN (SELECT * FROM test_table LIMIT <offset>, <size>)

Without this feature, the automatic pagination would generate the following SQL:

SELECT count(*) AS count FROM ( 
  SELECT * FROM test_table2 WHERE id IN (SELECT * FROM test_table) 
) AS t1 LIMIT 1;

SELECT * FROM ( 
  SELECT * FROM test_table2 WHERE id IN (SELECT * FROM test_table) 
) AS t1 LIMIT 10 OFFSET 0;
.
.

which is the pagination applied to the outer query instead of the inner one.

[andsel]

LGTM

[kares]

sorry for the late review, did you guys consider having better naming for the feature? e.g.

jdbc_paging_enabled => true jdbc_paging_mode => offset any chance this reads better (than "manual")?
(jdbc_paging_mode => count would de the default value)

should also keep things open for changing the paging strategy again (offset paging is a bit of an anti pattern although for mostly static data and LS' needs of at least once delivery it's sufficient), wdyt?

[andsel]

@kares I like your suggestion, instead of using a boolean flag defined a string value for the strategy is more readable.
However this feature doesn't cover only static data but also use cases where the Sequel automatic paging doesn't apply correctly, mostly:

• procedure calls with paging limits
• paging in inner queries in case of nested SQL queries

[kares]

I see, maybe we need better naming there - haven't thought about it that much.
however the manual just read a bit weird - had no idea what that could mean SQL wise.

what I did is check Sequel's each_page and it's using a COUNT(...) initial query (the details on how the statement is transformed into a count is usually adapter specific), haven't checked any adapter specific overrides for each_page.

[andsel]

@kares to me the naming you are proposing is good, better than the boolean flag. So I would use it.

[dingx1]

@kares @andsel @karenzone Thanks for your time. I also think this suggestion is good and will improve according to it.
I have a little question about the name.
Actually, this feature involves two options:

1. Not to add paging condition automatically, but explicitly customized by user.
2. Not to use count query first

The first one is the main purpose of this feature.
So, is it easy for users to understand the two values of 'count' and 'offset'?
If everyone don't have other suggested names, I will implement it according to current suggestion.

[andsel]

Good point, we could use

• automatic for dbc_paging_mode => count and
• explicit for jdbc_paging_mode => offset

@kares is it a better naming in your opinion?

[kares]

those sound better instead of automatic maybe jdbc_paging_mode => auto to choose the best available strategy based on the driver (Sequel does this). jdbc_paging_mode => explicit or manual both sounds okay to me.

[kares]

would be nice if there was a new class as it's a different algorithm e.g. ExplicitStatementHandler instead of hacking the algorithm into the existing one.

[andsel]

nice idea, I come up with some commits to switch this

[kares]

those sound better instead of automatic maybe jdbc_paging_mode => auto to choose the best available strategy based on the driver (Sequel does this). jdbc_paging_mode => explicit or manual both sounds okay to me.

[andsel]

@kares I've integrated your great latest suggest, I would ask another round of your eyes on this, please.

[kares]

LGTM, well done!

[andsel]

Many thanks @dingx1 for all of this 🥇 it was published with https://rubygems.org/gems/logstash-integration-jdbc/versions/5.2.0

[dingx1]

Wow, it's finally done. Great! ^_^
Thank you @andsel and everyone for your help！