Repository navigation
#822 Automatically detect JDBC session time zone when using a incremental offsets having 'datetime' type #823
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
Changes from all commits
dfe8736
91424ef
ca69169
1d0d8dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ import za.co.absa.pramen.core.utils.JdbcNativeUtils.JDBC_WORDS_TO_REDACT | |
| import za.co.absa.pramen.core.utils.{ConfigUtils, JdbcNativeUtils} | ||
|
|
||
| import java.time.LocalDate | ||
| import scala.util.{Failure, Success} | ||
|
|
||
| abstract class TableReaderJdbcBase(jdbcReaderConfig: TableReaderJdbcConfig, | ||
| jdbcUrlSelector: JdbcUrlSelector, | ||
|
|
@@ -43,18 +44,36 @@ abstract class TableReaderJdbcBase(jdbcReaderConfig: TableReaderJdbcConfig, | |
| val gen = SqlGeneratorLoader.getSqlGenerator(jdbcReaderConfig.jdbcConfig.driver, getSqlConfig) | ||
|
|
||
| if (gen.requiresConnection) { | ||
| val (connection, _) = jdbcUrlSelector.getNewConnection(jdbcRetries) | ||
| val (connection, _) = jdbcUrlSelector.getConnection | ||
| gen.setConnection(connection) | ||
| } | ||
| gen | ||
| } | ||
|
|
||
| private[core] lazy val serverTimeZone = { | ||
| jdbcReaderConfig.serverTimeZone match { | ||
| case Some(tz) => | ||
| log.info(s"The server timezone is specified explicitly: $tz") | ||
| tz | ||
| case None => | ||
| val (connection, _) = jdbcUrlSelector.getConnection | ||
| sqlGen.getServerTimezone(connection) match { | ||
|
Comment on lines
+59
to
+60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the selector's acquisition and retry contracts without running repository code.
fd 'JdbcUrlSelector.scala' . -x sh -c '
for file do
ast-grep outline "$file" --items all --match "getConnection|getNewConnection|getNextUrl"
rg -n -C 12 "def (getConnection|getNewConnection|getNextUrl)\b" "$file"
done
' shRepository: AbsaOSS/pramen Length of output: 2191 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- JdbcUrlSelector implementation ---'
rg -n -C 25 -F -- 'def getConnection' pramen/core/src/main/scala/za/co/absa/pramen/core/reader/JdbcUrlSelector.scala
printf '%s\n' '--- JdbcUrlSelector construction and connection helpers ---'
rg -n -C 12 -F -- 'class JdbcUrlSelectorImpl' pramen/core/src/main/scala/za/co/absa/pramen/core/reader/JdbcUrlSelector.scala
rg -n -C 18 -e 'getNewConnection' -e 'DriverManager.getConnection' pramen/core/src/main/scala/za/co/absa/pramen/core/reader/JdbcUrlSelector.scala
printf '%s\n' '--- TableReaderJdbcBase timezone block ---'
nl -ba pramen/core/src/main/scala/za/co/absa/pramen/core/reader/TableReaderJdbcBase.scala | sed -n '35,95p'
printf '%s\n' '--- timezone and selector call sites ---'
rg -n -C 8 -e 'serverTimeZone' -e 'jdbcUrlSelector.getConnection' pramen/core/src/main/scala/za/co/absa/pramen/core/readerRepository: AbsaOSS/pramen Length of output: 2135 🏁 Script executed: set -euo pipefail
nl -ba pramen/core/src/main/scala/za/co/absa/pramen/core/reader/JdbcUrlSelector.scala | sed -n '80,220p'
nl -ba pramen/core/src/main/scala/za/co/absa/pramen/core/reader/TableReaderJdbcBase.scala | sed -n '45,90p'Repository: AbsaOSS/pramen Length of output: 2443 🏁 Script executed: set -euo pipefail
rg --files pramen | rg 'JdbcUrlSelector'
printf '%s\n' '--- concrete implementation references ---'
rg -n -C 20 -e 'class JdbcUrlSelectorImpl' -e 'def getConnection' -e 'def getNewConnection' pramen/core/src/main/scala pramen/core/src/test/scalaRepository: AbsaOSS/pramen Length of output: 41800 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- TableReaderJdbcBase declarations and timezone usages ---'
nl -ba pramen/core/src/main/scala/za/co/absa/pramen/core/reader/TableReaderJdbcBase.scala | sed -n '1,55p'
rg -n -C 8 -F -- 'serverTimeZone' pramen/core/src/main/scala/za/co/absa/pramen/core/reader/TableReaderJdbcBase.scala
printf '%s\n' '--- selector terminal retry branch ---'
nl -ba pramen/core/src/main/scala/za/co/absa/pramen/core/reader/JdbcUrlSelectorImpl.scala | sed -n '145,180p'Repository: AbsaOSS/pramen Length of output: 4905 🏁 Script executed: set -euo pipefail
rg -n -F -- 'serverTimeZone' pramenRepository: AbsaOSS/pramen Length of output: 9194 Include connection acquisition in the timezone-detection fallback.
🤖 Prompt for AI Agents |
||
| case Success(tz) => | ||
| log.info(s"The server timezone is detected as: $tz") | ||
| tz | ||
| case Failure(ex) => | ||
| log.warn(s"Failed to detect the server timezone: '${ex.getMessage}'. Using the default timezone: ${jdbcReaderConfig.defaultTimeZone}") | ||
| jdbcReaderConfig.defaultTimeZone | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private[core] def getSqlConfig: SqlConfig = { | ||
| SqlConfig(jdbcReaderConfig.infoDateColumn, | ||
| jdbcReaderConfig.infoDateType, | ||
| jdbcReaderConfig.infoDateFormat, | ||
| jdbcReaderConfig.offsetInfoOpt, | ||
| jdbcReaderConfig.serverTimeZone, | ||
| jdbcReaderConfig.serverTimeZone.getOrElse(jdbcReaderConfig.defaultTimeZone), | ||
| jdbcReaderConfig.identifierQuotingPolicy, | ||
| jdbcReaderConfig.sqlGeneratorClass, | ||
| ConfigUtils.getExtraConfig(conf, "sql")) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the public
SqlGeneratorAPI compatible on thesupport/v1.14.12patch branch.SqlGeneratoris part ofpramen-api.SqlConfig.sqlGeneratorClasslets users plug in their own implementations. This change breaks those implementations in two ways:getServerTimezone(connection).getDataQueryIncremental.A custom generator compiled against the previous API fails in two ways:
AbstractMethodErrorwhen the reader callsgetServerTimezoneor the 6-argumentgetDataQueryIncremental.The same problem applies to subclasses of
SqlGeneratorBase, which must now implementgetServerTimezoneQuery. TheirgetOffsetWhereCondition(column, condition, offset)override no longer matches the new signature.Add default implementations so that existing generators keep working and fall back to the configured timezone.
♻️ Proposed compatible defaults
In
SqlGeneratorBase, givegetServerTimezoneQuerya default that throwsUnsupportedOperationException.getServerTimezonethen returns aFailure. Consider keeping the old 5-argumentgetDataQueryIncrementalas a concrete overload that delegates withNone.📝 Committable suggestion
🤖 Prompt for AI Agents