diff --git a/.github/workflows/style.yml b/.github/workflows/style.yml new file mode 100644 index 00000000..e79a40e9 --- /dev/null +++ b/.github/workflows/style.yml @@ -0,0 +1,35 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: "Check Style" + +on: + push: + branches: + - "master" + pull_request: + branches: + - "master" + +jobs: + check-style: + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-java@v2 + with: + distribution: zulu + java-version: 8 + cache: gradle + - run: >- + ./gradlew spotlessCheck --no-daemon --refresh-dependencies + -PmavenCentralMirror=https://maven-central.storage-download.googleapis.com/maven2/ diff --git a/.github/workflows/tpcds.yml b/.github/workflows/tpcds.yml index 3cafbd1a..3f30e012 100644 --- a/.github/workflows/tpcds.yml +++ b/.github/workflows/tpcds.yml @@ -11,6 +11,7 @@ # limitations under the License. name: "Build and Test" + on: push: branches: @@ -20,7 +21,7 @@ on: - "master" jobs: - tpcds-tests: + run-tpcds-sf1: runs-on: ubuntu-20.04 strategy: fail-fast: false diff --git a/.scalafmt.conf b/.scalafmt.conf index 163ffc5f..5c4ca2cc 100644 --- a/.scalafmt.conf +++ b/.scalafmt.conf @@ -10,16 +10,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -version = 3.1.2 +version=3.6.1 runner.dialect=scala212 project.git=true -align.preset = none -align.stripMargin = true -docstrings.style = Asterisk -maxColumn = 120 -newlines.source = keep -continuationIndent.defnSite = 2 -danglingParentheses.callSite = true -assumeStandardLibraryStripMargin = true -rewrite.rules = [SortImports, RedundantBraces, RedundantParens, SortModifiers] +align.preset=none +align.stripMargin=true +docstrings.style=keep +maxColumn=120 +newlines.source=keep +continuationIndent.defnSite=2 +danglingParentheses.callSite=true +assumeStandardLibraryStripMargin=true +rewrite.rules=[SortImports, RedundantBraces, RedundantParens, SortModifiers] diff --git a/build.gradle b/build.gradle index 785412cf..c3e0e8c7 100644 --- a/build.gradle +++ b/build.gradle @@ -16,6 +16,7 @@ plugins { id "idea" id "org.scoverage" version "7.0.1" apply false id "org.sonarqube" version "3.4.0.2513" + id "com.diffplug.spotless" version "6.12.1" apply false id "org.nosphere.apache.rat" version "0.8.0" id "com.github.maiflai.scalatest" version "0.32" apply false id "com.github.johnrengelman.shadow" version "7.1.2" apply false @@ -60,6 +61,7 @@ subprojects { apply plugin: "scala" apply plugin: "java-library" apply plugin: "org.scoverage" + apply plugin: "com.diffplug.spotless" apply plugin: "com.github.maiflai.scalatest" repositories { @@ -99,6 +101,14 @@ subprojects { highlighting.set(false) minimumRate.set(0.0) } + + spotless { + scala { + scalafmt("3.6.1") + .configFile("${rootProject.projectDir}/.scalafmt.conf") + .scalaMajorVersion("2.12") + } + } } project(':clickhouse-core') { diff --git a/clickhouse-core/src/main/scala/xenon/clickhouse/Utils.scala b/clickhouse-core/src/main/scala/xenon/clickhouse/Utils.scala index 415e0dbf..bdb68c0c 100644 --- a/clickhouse-core/src/main/scala/xenon/clickhouse/Utils.scala +++ b/clickhouse-core/src/main/scala/xenon/clickhouse/Utils.scala @@ -47,9 +47,8 @@ object Utils extends Logging { def classpathResourceAsStream(name: String): InputStream = defaultClassLoader.getResourceAsStream(name) - def getCodeSourceLocation(clazz: Class[_]): String = { + def getCodeSourceLocation(clazz: Class[_]): String = new File(clazz.getProtectionDomain.getCodeSource.getLocation.toURI).getPath - } @transient lazy val tmpDirPath: Path = Files.createTempDirectory("classpath_res_") @@ -113,7 +112,7 @@ object Utils extends Logging { // The number is too large, show it in scientific notation. BigDecimal(size, new MathContext(3, RoundingMode.HALF_UP)).toString() + " B" } else { - val (value, unit) = { + val (value, unit) = if (size >= 2 * EiB) { (BigDecimal(size) / EiB, "EiB") } else if (size >= 2 * PiB) { @@ -129,7 +128,6 @@ object Utils extends Logging { } else { (BigDecimal(size), "B") } - } "%.1f %s".formatLocal(Locale.US, value, unit) } } diff --git a/clickhouse-core/src/main/scala/xenon/clickhouse/client/ClusterClient.scala b/clickhouse-core/src/main/scala/xenon/clickhouse/client/ClusterClient.scala index 1a604474..00171737 100644 --- a/clickhouse-core/src/main/scala/xenon/clickhouse/client/ClusterClient.scala +++ b/clickhouse-core/src/main/scala/xenon/clickhouse/client/ClusterClient.scala @@ -42,8 +42,8 @@ class ClusterClient(cluster: ClusterSpec) extends AutoCloseable with Logging { val replicaSpec = shuffle(shardSpec.replicas.toSeq).head (shardSpec.num, replicaSpec.num) case _ => throw CHClientException( - s"Invalid shard[${shard.orNull}] replica[${replica.orNull}] of cluster ${cluster.name}" - ) + s"Invalid shard[${shard.orNull}] replica[${replica.orNull}] of cluster ${cluster.name}" + ) } cache.computeIfAbsent( diff --git a/clickhouse-core/src/main/scala/xenon/clickhouse/parse/AstVisitor.scala b/clickhouse-core/src/main/scala/xenon/clickhouse/parse/AstVisitor.scala index 007c3f96..feb1f256 100644 --- a/clickhouse-core/src/main/scala/xenon/clickhouse/parse/AstVisitor.scala +++ b/clickhouse-core/src/main/scala/xenon/clickhouse/parse/AstVisitor.scala @@ -169,19 +169,21 @@ class AstVisitor extends ClickHouseSQLBaseVisitor[AnyRef] with Logging { TupleExpr(visitColumnExprList(ctx.columnExprList)) override def visitColumnExprPrecedence1(ctx: ColumnExprPrecedence1Context): FuncExpr = { - val funcName = if (ctx.PERCENT != null) "remainder" - else if (ctx.SLASH != null) "divide" - else if (ctx.ASTERISK != null) "multiply" - else throw new IllegalArgumentException(s"Invalid [ColumnExprPrecedence1] ${ctx.getText}") + val funcName = + if (ctx.PERCENT != null) "remainder" + else if (ctx.SLASH != null) "divide" + else if (ctx.ASTERISK != null) "multiply" + else throw new IllegalArgumentException(s"Invalid [ColumnExprPrecedence1] ${ctx.getText}") val funArgs = List(visitColumnExpr(ctx.columnExpr(0)), visitColumnExpr(ctx.columnExpr(1))) FuncExpr(funcName, funArgs) } override def visitColumnExprPrecedence2(ctx: ColumnExprPrecedence2Context): FuncExpr = { - val funcName = if (ctx.PLUS != null) "add" - else if (ctx.DASH != null) "subtract" - else if (ctx.CONCAT != null) "concat" - else throw new IllegalArgumentException(s"Invalid [ColumnExprPrecedence2] ${ctx.getText}") + val funcName = + if (ctx.PLUS != null) "add" + else if (ctx.DASH != null) "subtract" + else if (ctx.CONCAT != null) "concat" + else throw new IllegalArgumentException(s"Invalid [ColumnExprPrecedence2] ${ctx.getText}") val funArgs = List(visitColumnExpr(ctx.columnExpr(0)), visitColumnExpr(ctx.columnExpr(1))) FuncExpr(funcName, funArgs) } diff --git a/clickhouse-core/src/main/scala/xenon/clickhouse/parse/SQLParser.scala b/clickhouse-core/src/main/scala/xenon/clickhouse/parse/SQLParser.scala index 99c76a17..a11ba742 100644 --- a/clickhouse-core/src/main/scala/xenon/clickhouse/parse/SQLParser.scala +++ b/clickhouse-core/src/main/scala/xenon/clickhouse/parse/SQLParser.scala @@ -42,20 +42,22 @@ class SQLParser(astVisitor: AstVisitor) extends Logging { parser.removeErrorListeners() parser.addErrorListener(ParseErrorListener) - try try { - // first, try parsing with potentially faster SLL mode - parser.getInterpreter.setPredictionMode(PredictionMode.SLL) - toResult(parser) - } catch { - case _: ParseCancellationException => - // if we fail, parse with LL mode - tokenStream.seek(0) // rewind input stream - parser.reset() - - // Try Again. - parser.getInterpreter.setPredictionMode(PredictionMode.LL) + try + try { + // first, try parsing with potentially faster SLL mode + parser.getInterpreter.setPredictionMode(PredictionMode.SLL) toResult(parser) - } catch { + } catch { + case _: ParseCancellationException => + // if we fail, parse with LL mode + tokenStream.seek(0) // rewind input stream + parser.reset() + + // Try Again. + parser.getInterpreter.setPredictionMode(PredictionMode.LL) + toResult(parser) + } + catch { case rethrow: ParseException => throw rethrow } } diff --git a/dev/reformat b/dev/reformat new file mode 100755 index 00000000..3891c2fc --- /dev/null +++ b/dev/reformat @@ -0,0 +1,19 @@ +#!/bin/bash +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +set -x + +PROJECT_DIR="$(cd "`dirname "$0"`/.."; pwd)" +${PROJECT_DIR}/gradlew spotlessApply diff --git a/spark-3.3/clickhouse-spark-it/src/test/scala/org/apache/spark/sql/clickhouse/SparkTest.scala b/spark-3.3/clickhouse-spark-it/src/test/scala/org/apache/spark/sql/clickhouse/SparkTest.scala index 6adabbbd..3c7bfb24 100644 --- a/spark-3.3/clickhouse-spark-it/src/test/scala/org/apache/spark/sql/clickhouse/SparkTest.scala +++ b/spark-3.3/clickhouse-spark-it/src/test/scala/org/apache/spark/sql/clickhouse/SparkTest.scala @@ -45,9 +45,9 @@ trait SparkTest extends QueryTest with SharedSparkSession { spark.sql(s"CREATE DATABASE IF NOT EXISTS `$database`") block(database, table) } finally if (cleanup) { - spark.sql(s"DROP TABLE IF EXISTS `$database`.`$table`") - spark.sql(s"DROP DATABASE IF EXISTS `$database` CASCADE") - } + spark.sql(s"DROP TABLE IF EXISTS `$database`.`$table`") + spark.sql(s"DROP DATABASE IF EXISTS `$database` CASCADE") + } def withClickHouseSingleIdTable( database: String, diff --git a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/ClickHouseHelper.scala b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/ClickHouseHelper.scala index 962d2002..f39bfb1c 100644 --- a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/ClickHouseHelper.scala +++ b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/ClickHouseHelper.scala @@ -15,7 +15,6 @@ package xenon.clickhouse import com.clickhouse.client.ClickHouseProtocol -import com.clickhouse.client.config.ClickHouseClientOption import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.node.NullNode import org.apache.spark.sql.catalyst.analysis.{NoSuchNamespaceException, NoSuchTableException} diff --git a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/InputPartitions.scala b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/InputPartitions.scala index 1d415be5..bcfde4fb 100644 --- a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/InputPartitions.scala +++ b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/InputPartitions.scala @@ -46,12 +46,11 @@ case class ClickHouseInputPartition( } // TODO improve and test - def compilePartitionFilterValue(partitionValue: String): String = { + def compilePartitionFilterValue(partitionValue: String): String = (partitionValue.contains("-"), partitionValue.contains("(")) match { // quote when partition by a single Date Type column to avoid illegal types of arguments (Date, Int64) case (true, false) => s"'$partitionValue'" // Date type column is quoted if there are multi partition columns case _ => s"$partitionValue" } - } } diff --git a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/format/ClickHouseJsonReader.scala b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/format/ClickHouseJsonReader.scala index 89ccb7ea..d81b5ece 100644 --- a/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/format/ClickHouseJsonReader.scala +++ b/spark-3.3/clickhouse-spark/src/main/scala/xenon/clickhouse/read/format/ClickHouseJsonReader.scala @@ -36,9 +36,8 @@ class ClickHouseJsonReader( override val format: String = "JSONCompactEachRowWithNamesAndTypes" - lazy val streamOutput: StreamOutput[Array[JsonNode]] = { + lazy val streamOutput: StreamOutput[Array[JsonNode]] = JSONCompactEachRowWithNamesAndTypesStreamOutput.deserializeStream(resp.getInputStream) - } override def decode(record: Array[JsonNode]): InternalRow = { val values: Array[Any] = new Array[Any](record.length) diff --git a/spark-3.3/clickhouse-spark/src/test/scala/org/apache/spark/sql/clickhouse/ConfigurationSuite.scala b/spark-3.3/clickhouse-spark/src/test/scala/org/apache/spark/sql/clickhouse/ConfigurationSuite.scala index a2e8170a..2fd17e94 100644 --- a/spark-3.3/clickhouse-spark/src/test/scala/org/apache/spark/sql/clickhouse/ConfigurationSuite.scala +++ b/spark-3.3/clickhouse-spark/src/test/scala/org/apache/spark/sql/clickhouse/ConfigurationSuite.scala @@ -107,9 +107,10 @@ class ConfigurationSuite extends AnyFunSuite { StandardOpenOption.CREATE ) try newOutput.foreach { line => - writer.write(line) - writer.newLine() - } finally writer.close() + writer.write(line) + writer.newLine() + } + finally writer.close() } else { val expected = Files.readAllLines(goldenFile).asScala val hint = s"$goldenFile is out of date, please update the golden file with " +