You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/11/16 07:55:01 UTC
[GitHub] [beam] vitaly-ivanov opened a new pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
vitaly-ivanov opened a new pull request #15987:
URL: https://github.com/apache/beam/pull/15987
This is a fix for IllegalArgumentException that happens during reading of a numeric column. The issue is following: say you have a numeric field with precision 15 and scale 2. When JdbcIO reads values they might have smaller precision, say 6. I loosen the check so if the precision and the scale are less or equal than defined for a column we allow it.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-970003876
R: @jbonofre @timrobertson100
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r753675113
##########
File path: sdks/java/io/jdbc/src/test/java/org/apache/beam/sdk/io/jdbc/JdbcIOTest.java
##########
@@ -309,6 +310,40 @@ public void testReadRowsWithNumericFields() {
pipeline.run();
}
+ @Test
+ public void testReadRowsWithNumericFieldsWithExcessPrecision() {
+ PCollection<Row> rows =
+ pipeline.apply(
+ JdbcIO.readRows()
+ .withDataSourceConfiguration(DATA_SOURCE_CONFIGURATION)
+ .withQuery(
+ String.format(
+ "SELECT CAST(1 AS NUMERIC(10, 2)) AS T1 FROM %s WHERE name = ?",
+ READ_TABLE_NAME))
+ .withStatementPreparator(
+ preparedStatement ->
+ preparedStatement.setString(1, TestRow.getNameForSeed(1))));
+
+ Schema expectedSchema =
+ Schema.of(
+ Schema.Field.of(
+ "T1",
+ FieldType.logicalType(FixedPrecisionNumeric.of(NUMERIC.getName(), 10, 2))
Review comment:
shouldn't this be testing a precision / scale that is larger than the numeric in line 321?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] timrobertson100 commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
timrobertson100 commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-989802457
Hi @Pavlmits
Beam releases every 6 weeks, so there should be a release in January. In the meantime, perhaps you could apply the changes of this PR into your own project by creating the`org/apache/beam/sdk/io/jdbc/LogicalTypes.java` class and copying the contents of [this file](https://github.com/vitaly-ivanov/beam/blob/9fa3bcaf724a9be53775af3d838154228022c9e2/sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java) in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] borovikovd commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
borovikovd commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r751686195
##########
File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java
##########
@@ -266,8 +266,8 @@ private FixedPrecisionNumeric(
@Override
public BigDecimal toInputType(BigDecimal base) {
checkArgument(
- base == null || (base.precision() == precision && base.scale() == scale),
- "Expected BigDecimal base to be null or have precision = %s (was %s), scale = %s (was %s)",
+ base == null || (base.precision() <= precision && base.scale() <= scale),
Review comment:
@kennknowles would
`(base.precision() - base.scale()) <= (precision - scale) && (base.scale() <= scale)`
work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r751524811
##########
File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java
##########
@@ -266,8 +266,8 @@ private FixedPrecisionNumeric(
@Override
public BigDecimal toInputType(BigDecimal base) {
checkArgument(
- base == null || (base.precision() == precision && base.scale() == scale),
- "Expected BigDecimal base to be null or have precision = %s (was %s), scale = %s (was %s)",
+ base == null || (base.precision() <= precision && base.scale() <= scale),
Review comment:
Incidentally it took me a minute to understand what is happening here because I don't know why it is called `base`. This is converting the value returned by JDBC to the value expected by Beam schema, right?
I am having trouble wrapping my head around all the cases here, or coming up with the simple math. The inefficient but simple check here is `base.round(new MathContext(precision)).compareTo(base) == 0`.
If the scales are equal, then lower precision is fine, certainly.
But you could even have "1000 x 10^-3" (precision 3, scale 3) and it can safely coerce to "1 x 10^0" (precision 1, scale 0). Am I getting this wrong?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Pavlmits commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
Pavlmits commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-989782555
Hello people,
I am facing the same issue. When this feature will be released? Thank you in advance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-982714106
thanks LGTM. I had hit this issue at some point before and didn't take the time to fix it, so I appreciate you looking into it : )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r753675113
##########
File path: sdks/java/io/jdbc/src/test/java/org/apache/beam/sdk/io/jdbc/JdbcIOTest.java
##########
@@ -309,6 +310,40 @@ public void testReadRowsWithNumericFields() {
pipeline.run();
}
+ @Test
+ public void testReadRowsWithNumericFieldsWithExcessPrecision() {
+ PCollection<Row> rows =
+ pipeline.apply(
+ JdbcIO.readRows()
+ .withDataSourceConfiguration(DATA_SOURCE_CONFIGURATION)
+ .withQuery(
+ String.format(
+ "SELECT CAST(1 AS NUMERIC(10, 2)) AS T1 FROM %s WHERE name = ?",
+ READ_TABLE_NAME))
+ .withStatementPreparator(
+ preparedStatement ->
+ preparedStatement.setString(1, TestRow.getNameForSeed(1))));
+
+ Schema expectedSchema =
+ Schema.of(
+ Schema.Field.of(
+ "T1",
+ FieldType.logicalType(FixedPrecisionNumeric.of(NUMERIC.getName(), 10, 2))
Review comment:
shouldn't this be testing a precision / scale that is larger than the numeric in line 321?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-974221266
thanks for working on this! I had hit this issue a little while earlier, and I'm happy to see it getting fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] borovikovd commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
borovikovd commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r751686195
##########
File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java
##########
@@ -266,8 +266,8 @@ private FixedPrecisionNumeric(
@Override
public BigDecimal toInputType(BigDecimal base) {
checkArgument(
- base == null || (base.precision() == precision && base.scale() == scale),
- "Expected BigDecimal base to be null or have precision = %s (was %s), scale = %s (was %s)",
+ base == null || (base.precision() <= precision && base.scale() <= scale),
Review comment:
@kennknowles would
`(base.precision() - base.scale()) <= (precision - scale) && base.scale() <= scale`
work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-974221507
I'll take a look today. I need to wrap my head around the code first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r753689200
##########
File path: sdks/java/io/jdbc/src/test/java/org/apache/beam/sdk/io/jdbc/JdbcIOTest.java
##########
@@ -309,6 +310,40 @@ public void testReadRowsWithNumericFields() {
pipeline.run();
}
+ @Test
+ public void testReadRowsWithNumericFieldsWithExcessPrecision() {
+ PCollection<Row> rows =
+ pipeline.apply(
+ JdbcIO.readRows()
+ .withDataSourceConfiguration(DATA_SOURCE_CONFIGURATION)
+ .withQuery(
+ String.format(
+ "SELECT CAST(1 AS NUMERIC(10, 2)) AS T1 FROM %s WHERE name = ?",
+ READ_TABLE_NAME))
+ .withStatementPreparator(
+ preparedStatement ->
+ preparedStatement.setString(1, TestRow.getNameForSeed(1))));
+
+ Schema expectedSchema =
+ Schema.of(
+ Schema.Field.of(
+ "T1",
+ FieldType.logicalType(FixedPrecisionNumeric.of(NUMERIC.getName(), 10, 2))
Review comment:
It checks the case where we have a column with excess precision (10) and a value with a smaller precision (3). Perhaps the name of the test is a bit confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r751993295
##########
File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java
##########
@@ -266,8 +266,8 @@ private FixedPrecisionNumeric(
@Override
public BigDecimal toInputType(BigDecimal base) {
checkArgument(
- base == null || (base.precision() == precision && base.scale() == scale),
- "Expected BigDecimal base to be null or have precision = %s (was %s), scale = %s (was %s)",
+ base == null || (base.precision() <= precision && base.scale() <= scale),
Review comment:
I think we can add smth like
```java
base == null || (base.precision() <= precision && base.scale() <= scale)
// for cases when received values can be safely coerced to the schema
|| base.round(new MathContext(precision)).compareTo(base) == 0
```
I suppose, the first condition will be enough for the vast majority of cases, when the second one will be executed for negative cases and cases when values can be safely coerced to the schema.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem merged pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #15987:
URL: https://github.com/apache/beam/pull/15987
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] timrobertson100 edited a comment on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
timrobertson100 edited a comment on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-989802457
Hi @Pavlmits
Beam releases every ~6 weeks, so there should be a release in January. In the meantime, perhaps you could apply the changes of this PR into your own project by creating the`org/apache/beam/sdk/io/jdbc/LogicalTypes.java` class and copying the contents of [this file](https://github.com/vitaly-ivanov/beam/blob/9fa3bcaf724a9be53775af3d838154228022c9e2/sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java) in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Pavlmits commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
Pavlmits commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-989828179
Thank you very much @timrobertson100! It worked! :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] borovikovd commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
borovikovd commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-980254828
@pabloem any update on this?:) We have to use our own custom build to be able to use Beam until this merged...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-970152259
retest this please
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r751966250
##########
File path: sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/LogicalTypes.java
##########
@@ -266,8 +266,8 @@ private FixedPrecisionNumeric(
@Override
public BigDecimal toInputType(BigDecimal base) {
checkArgument(
- base == null || (base.precision() == precision && base.scale() == scale),
- "Expected BigDecimal base to be null or have precision = %s (was %s), scale = %s (was %s)",
+ base == null || (base.precision() <= precision && base.scale() <= scale),
Review comment:
@borovikovd Looks like it doesn't work with @kennknowles's example because `base.scale()` (3) > `scale` (0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] vitaly-ivanov commented on a change in pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
vitaly-ivanov commented on a change in pull request #15987:
URL: https://github.com/apache/beam/pull/15987#discussion_r753689200
##########
File path: sdks/java/io/jdbc/src/test/java/org/apache/beam/sdk/io/jdbc/JdbcIOTest.java
##########
@@ -309,6 +310,40 @@ public void testReadRowsWithNumericFields() {
pipeline.run();
}
+ @Test
+ public void testReadRowsWithNumericFieldsWithExcessPrecision() {
+ PCollection<Row> rows =
+ pipeline.apply(
+ JdbcIO.readRows()
+ .withDataSourceConfiguration(DATA_SOURCE_CONFIGURATION)
+ .withQuery(
+ String.format(
+ "SELECT CAST(1 AS NUMERIC(10, 2)) AS T1 FROM %s WHERE name = ?",
+ READ_TABLE_NAME))
+ .withStatementPreparator(
+ preparedStatement ->
+ preparedStatement.setString(1, TestRow.getNameForSeed(1))));
+
+ Schema expectedSchema =
+ Schema.of(
+ Schema.Field.of(
+ "T1",
+ FieldType.logicalType(FixedPrecisionNumeric.of(NUMERIC.getName(), 10, 2))
Review comment:
It checks the case where we have a column with excess precision (10) and a value with a smaller precision (3). Perhaps the name of the test is a bit confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] timrobertson100 commented on pull request #15987: [BEAM-13242] Allow values with smaller precision and scale for FixedP…
Posted by GitBox <gi...@apache.org>.
timrobertson100 commented on pull request #15987:
URL: https://github.com/apache/beam/pull/15987#issuecomment-989866477
Very welcome, and happy to hear it worked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org