You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by ro...@apache.org on 2022/12/02 23:52:09 UTC
[beam] branch master updated: [SQL extension] Minor fixes to logging applying best practices (#24328)
This is an automated email from the ASF dual-hosted git repository.
robertwb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new 62964b48957 [SQL extension] Minor fixes to logging applying best practices (#24328)
62964b48957 is described below
commit 62964b489571548cbc413b08b65588d334a81d7c
Author: Moritz Mack <mm...@talend.com>
AuthorDate: Sat Dec 3 00:52:02 2022 +0100
[SQL extension] Minor fixes to logging applying best practices (#24328)
---
.../meta/provider/datacatalog/DataCatalogTableProvider.java | 2 +-
.../beam/sdk/extensions/sql/impl/CalciteQueryPlanner.java | 9 ++++-----
.../beam/sdk/extensions/sql/impl/rel/BeamSqlRelUtils.java | 11 +++++++++++
.../extensions/sql/meta/provider/bigquery/BigQueryTable.java | 6 +++---
.../extensions/sql/meta/provider/mongodb/MongoDbTable.java | 2 +-
.../extensions/sql/meta/provider/parquet/ParquetTable.java | 2 +-
.../beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java | 4 ++--
7 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogTableProvider.java b/sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogTableProvider.java
index 4ccc32516d4..60c224f4cae 100644
--- a/sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogTableProvider.java
+++ b/sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogTableProvider.java
@@ -229,7 +229,7 @@ public class DataCatalogTableProvider extends FullNameTableProvider implements A
.build());
return true;
} else {
- LOG.info(String.format("Not updating schema for '%s' since it already has one.", resource));
+ LOG.info("Not updating schema for '{}' since it already has one.", resource);
return false;
}
}
diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/CalciteQueryPlanner.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/CalciteQueryPlanner.java
index f4dd89bd349..d0515969c52 100644
--- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/CalciteQueryPlanner.java
+++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/CalciteQueryPlanner.java
@@ -29,6 +29,7 @@ import org.apache.beam.sdk.extensions.sql.impl.planner.BeamRelMetadataQuery;
import org.apache.beam.sdk.extensions.sql.impl.planner.RelMdNodeStats;
import org.apache.beam.sdk.extensions.sql.impl.rel.BeamLogicalConvention;
import org.apache.beam.sdk.extensions.sql.impl.rel.BeamRelNode;
+import org.apache.beam.sdk.extensions.sql.impl.rel.BeamSqlRelUtils;
import org.apache.beam.sdk.extensions.sql.impl.udf.BeamBuiltinFunctionProvider;
import org.apache.beam.vendor.calcite.v1_28_0.com.google.common.collect.ImmutableList;
import org.apache.beam.vendor.calcite.v1_28_0.com.google.common.collect.Table;
@@ -38,7 +39,6 @@ import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.Contexts;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.ConventionTraitDef;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptCost;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptPlanner.CannotPlanException;
-import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptUtil;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelTraitDef;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelTraitSet;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.prepare.CalciteCatalogReader;
@@ -182,12 +182,11 @@ public class CalciteQueryPlanner implements QueryPlanner {
SqlNode parsed = planner.parse(sqlStatement);
TableResolutionUtils.setupCustomTableResolution(connection, parsed);
SqlNode validated = planner.validate(parsed);
- LOG.info("SQL:\n" + validated);
+ LOG.info("SQL:\n{}", validated);
// root of original logical plan
RelRoot root = planner.rel(validated);
- LOG.info("SQLPlan>\n" + RelOptUtil.toString(root.rel));
-
+ LOG.info("SQLPlan>\n{}", BeamSqlRelUtils.explainLazily(root.rel));
RelTraitSet desiredTraits =
root.rel
.getTraitSet()
@@ -209,7 +208,7 @@ public class CalciteQueryPlanner implements QueryPlanner {
JaninoRelMetadataProvider.of(root.rel.getCluster().getMetadataProvider()));
root.rel.getCluster().invalidateMetadataQuery();
beamRelNode = (BeamRelNode) planner.transform(0, desiredTraits, root.rel);
- LOG.info("BEAMPlan>\n" + RelOptUtil.toString(beamRelNode));
+ LOG.info("BEAMPlan>\n{}", BeamSqlRelUtils.explainLazily(beamRelNode));
} catch (RelConversionException | CannotPlanException e) {
throw new SqlConversionException(
String.format("Unable to convert query %s", sqlStatement), e);
diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSqlRelUtils.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSqlRelUtils.java
index 0086e73c364..8bcb8389e56 100644
--- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSqlRelUtils.java
+++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSqlRelUtils.java
@@ -31,6 +31,7 @@ import org.apache.beam.sdk.values.PCollection;
import org.apache.beam.sdk.values.PCollectionList;
import org.apache.beam.sdk.values.POutput;
import org.apache.beam.sdk.values.Row;
+import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptUtil;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.volcano.RelSubset;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.rel.RelNode;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -145,4 +146,14 @@ public class BeamSqlRelUtils {
Schema.Field.of(ROW, Schema.FieldType.row(upstreamSchema)),
Schema.Field.of(ERROR, Schema.FieldType.STRING));
}
+
+ /** A lazy explain via {@link #toString()} for logging purposes. */
+ public static Object explainLazily(final RelNode node) {
+ return new Object() {
+ @Override
+ public String toString() {
+ return RelOptUtil.toString(node);
+ }
+ };
+ }
}
diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryTable.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryTable.java
index b42edb2dcfa..c450734ec36 100644
--- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryTable.java
+++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryTable.java
@@ -105,7 +105,7 @@ class BigQueryTable extends SchemaBaseBeamTable implements Serializable {
method = Method.DIRECT_READ;
}
- LOG.info("BigQuery method is set to: " + method.toString());
+ LOG.info("BigQuery method is set to: {}", method);
if (table.getProperties().containsKey(WRITE_DISPOSITION_PROPERTY)) {
List<String> validWriteDispositions =
@@ -130,7 +130,7 @@ class BigQueryTable extends SchemaBaseBeamTable implements Serializable {
writeDisposition = WriteDisposition.WRITE_EMPTY;
}
- LOG.info("BigQuery writeDisposition is set to: " + writeDisposition.toString());
+ LOG.info("BigQuery writeDisposition is set to: {}", writeDisposition);
}
@Override
@@ -172,7 +172,7 @@ class BigQueryTable extends SchemaBaseBeamTable implements Serializable {
if (!bigQueryFilter.getSupported().isEmpty()) {
String rowRestriction = generateRowRestrictions(getSchema(), bigQueryFilter.getSupported());
if (!rowRestriction.isEmpty()) {
- LOG.info("Pushing down the following filter: " + rowRestriction);
+ LOG.info("Pushing down the following filter: {}", rowRestriction);
typedRead = typedRead.withRowRestriction(rowRestriction);
}
}
diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java
index 3366d9ad988..59d52d79e03 100644
--- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java
+++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java
@@ -135,7 +135,7 @@ public class MongoDbTable extends SchemaBaseBeamTable implements Serializable {
MongoDbFilter mongoFilter = (MongoDbFilter) filters;
if (!mongoFilter.getSupported().isEmpty()) {
Bson filter = constructPredicate(mongoFilter.getSupported());
- LOG.info("Pushing down the following filter: " + filter.toString());
+ LOG.info("Pushing down the following filter: {}", filter);
findQuery = findQuery.withFilters(filter);
}
}
diff --git a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java
index e0d840c6d52..c38ed0ccf68 100644
--- a/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java
+++ b/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java
@@ -68,7 +68,7 @@ class ParquetTable extends SchemaBaseBeamTable implements Serializable {
Read read = ParquetIO.read(schema).withBeamSchemas(true).from(table.getLocation() + "/*");
if (!fieldNames.isEmpty()) {
Schema projectionSchema = projectSchema(schema, fieldNames);
- LOG.info("Projecting fields schema : " + projectionSchema.toString());
+ LOG.info("Projecting fields schema: {}", projectionSchema);
read = read.withProjection(projectionSchema, projectionSchema);
}
return begin.apply("ParquetIORead", read).apply("ToRows", Convert.toRows());
diff --git a/sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java b/sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
index ecd8af1fe16..482e14f12e0 100644
--- a/sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
+++ b/sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSQLQueryPlanner.java
@@ -34,6 +34,7 @@ import org.apache.beam.sdk.extensions.sql.impl.planner.BeamRuleSets;
import org.apache.beam.sdk.extensions.sql.impl.planner.RelMdNodeStats;
import org.apache.beam.sdk.extensions.sql.impl.rel.BeamLogicalConvention;
import org.apache.beam.sdk.extensions.sql.impl.rel.BeamRelNode;
+import org.apache.beam.sdk.extensions.sql.impl.rel.BeamSqlRelUtils;
import org.apache.beam.sdk.extensions.sql.impl.rule.BeamCalcRule;
import org.apache.beam.sdk.extensions.sql.impl.rule.BeamUncollectRule;
import org.apache.beam.sdk.extensions.sql.impl.rule.BeamUnnestRule;
@@ -44,7 +45,6 @@ import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.jdbc.CalciteSch
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.ConventionTraitDef;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptPlanner;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptRule;
-import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelOptUtil;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelTraitDef;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.plan.RelTraitSet;
import org.apache.beam.vendor.calcite.v1_28_0.org.apache.calcite.prepare.CalciteCatalogReader;
@@ -220,7 +220,7 @@ public class ZetaSQLQueryPlanner implements QueryPlanner {
root.rel.getCluster().invalidateMetadataQuery();
try {
BeamRelNode beamRelNode = (BeamRelNode) plannerImpl.transform(0, desiredTraits, root.rel);
- LOG.info("BEAMPlan>\n" + RelOptUtil.toString(beamRelNode));
+ LOG.info("BEAMPlan>\n{}", BeamSqlRelUtils.explainLazily(beamRelNode));
return beamRelNode;
} catch (RelOptPlanner.CannotPlanException e) {
throw new SqlConversionException("Failed to produce plan for query " + sql, e);