Parameterize segment IDs

[abhishekrb19]

Issue:

The markUsed API accepts either an interval or segmentIds. When using segment IDs, the underlying query uses an IN clause with the user-provided values inlined directly in the query instead of being parameterized.

Fix:

This patch fixes that by adding the following utilities specifically for queries that contain an IN clause:

• getParameterizedInConditionForColumn()
• bindColumnValuesToQueryWithInCondition()

Other related changes:

Renamed the following functions for consistency and code hygiene:

• appendConditionForIntervalsAndMatchMode() to getConditionForIntervalsAndMatchMode(). Pass only select arguments to the function.
• bindQueryIntervals() to bindIntervalsToQuery()

Release note:

The segmentIds filter in the markUsed API payload is parameterized in the DB query.

This PR has:

• been self-reviewed.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• been tested in a test Druid cluster.

[kfaraz]

Thanks for the change, @abhishekrb19 . This is very useful!

It would be nice to have a test to verify that the code throws an exception when passing illegal values for the version or any other column.

[abhishekrb19]

@kfaraz, thanks for taking a look!

Thanks for the change, @abhishekrb19 . This is very useful!

It would be nice to have a test to verify that the code throws an exception when passing illegal values for the version or any other column.

What kind of validation and exception did you have in mind? Currently, the code doesn't perform any type checks for the columns.

[kfaraz]

What kind of validation and exception did you have in mind? Currently, the code doesn't perform any type checks for the columns.

I should think that JDBI would throw some exception if we try to bind am invalid string which contains, say a substring like '); DROP TABLE abc;.
We could try to verify that behaviour. By the way, this is optional and need not block this PR.

[abhishekrb19]

Ah, I see. The JDBI library doesn't check for "illegal" values for the named parameters as it's contextual. I ran a test with such values and it seems the library just treats them as any other string literal. That said, the binding itself makes the query safe to execute and I think if we want to detect illegal values or perform semantic validation, it has to be done at a layer on top (I'm not sure if that check can be full-proof though).

[kfaraz]

That said, the binding itself makes the query safe to execute

Yeah, that's a fair point. Thanks for the clarification.