You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2020/01/06 08:11:58 UTC

[calcite] branch master updated (5e4d396 -> c416c31)

This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git.


    from 5e4d396  [CALCITE-3672] Support implicit type coercion for insert and update
     new 8658804  In DiffRepository, replace a synchronized Map with a Guava cache
     new 4e7f925  Add gradle task 'aggregateJavadocIncludingTests' that builds javadoc for both main and test
     new 8a49c9f  Bump spark-core_2.10 from 2.2.0 to 2.2.2
     new 61da868  Contribute class Resources to ASF, and change its header
     new 68eacfc  Javadoc warning
     new e88809e  [CALCITE-3328] Immutable beans, powered by reflection
     new c416c31  [CALCITE-3526] SqlPrettyWriter should have options to fold/chop long lines, print leading commas

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/calcite/test/BabelQuidemTest.java   |    4 +-
 babel/src/test/resources/sql/select.iq             |    2 +-
 build.gradle.kts                                   |   14 +
 .../calcite/adapter/cassandra/CassandraSchema.java |   13 +-
 .../org/apache/calcite/adapter/jdbc/JdbcTable.java |   12 +-
 .../calcite/rel/rel2sql/RelToSqlConverter.java     |    3 +-
 .../java/org/apache/calcite/runtime/Resources.java |   87 +-
 .../java/org/apache/calcite/sql/SqlAsOperator.java |    2 +-
 .../main/java/org/apache/calcite/sql/SqlJoin.java  |   20 +-
 .../org/apache/calcite/sql/SqlMatchRecognize.java  |    6 +-
 .../main/java/org/apache/calcite/sql/SqlNode.java  |   44 +-
 .../java/org/apache/calcite/sql/SqlNodeList.java   |   57 +-
 .../java/org/apache/calcite/sql/SqlOperator.java   |   36 +-
 .../java/org/apache/calcite/sql/SqlOrderBy.java    |    6 +-
 .../org/apache/calcite/sql/SqlSelectOperator.java  |   65 +-
 .../apache/calcite/sql/SqlWithinGroupOperator.java |    2 +-
 .../java/org/apache/calcite/sql/SqlWriter.java     |   55 +-
 .../org/apache/calcite/sql/SqlWriterConfig.java    |  434 +++++++
 .../apache/calcite/sql/fun/SqlCaseOperator.java    |    5 +-
 .../calcite/sql/fun/SqlStdOperatorTable.java       |    1 +
 .../apache/calcite/sql/pretty/SqlPrettyWriter.java | 1231 ++++++++++++--------
 .../apache/calcite/sql/type/IntervalSqlType.java   |   11 +-
 .../org/apache/calcite/util/ImmutableBeans.java    |  386 ++++++
 .../apache/calcite/sql/parser/SqlParserTest.java   |  168 ++-
 .../calcite/sql/test/SqlOperatorBaseTest.java      |   13 +-
 .../calcite/sql/test/SqlPrettyWriterTest.java      |  262 ++++-
 .../org/apache/calcite/test/DiffRepository.java    |   62 +-
 .../org/apache/calcite/test/JdbcAdapterTest.java   |    2 +-
 .../org/apache/calcite/test/RexProgramTest.java    |   11 +-
 .../apache/calcite/test/SqlToRelConverterTest.java |   26 +-
 .../test/enumerable/EnumerableCalcTest.java        |    3 +-
 .../calcite/test/fuzzer/RexProgramFuzzyTest.java   |   11 +-
 .../org/apache/calcite/util/ImmutableBeanTest.java |  433 +++++++
 .../calcite/sql/test/SqlPrettyWriterTest.xml       |  337 +++++-
 .../adapter/elasticsearch/PredicateAnalyzer.java   |    1 +
 .../calcite/adapter/elasticsearch/MatchTest.java   |   80 +-
 gradle.properties                                  |    2 +-
 .../org/apache/calcite/piglet/PigConverter.java    |   13 +-
 .../java/org/apache/calcite/test/PigRelOpTest.java |  122 +-
 .../org/apache/calcite/adapter/tpch/TpchTest.java  |    5 +-
 .../org/apache/calcite/sql/ddl/SqlDdlNodes.java    |   22 +-
 41 files changed, 3084 insertions(+), 985 deletions(-)
 create mode 100644 core/src/main/java/org/apache/calcite/sql/SqlWriterConfig.java
 create mode 100644 core/src/main/java/org/apache/calcite/util/ImmutableBeans.java
 create mode 100644 core/src/test/java/org/apache/calcite/util/ImmutableBeanTest.java


[calcite] 06/07: [CALCITE-3328] Immutable beans, powered by reflection

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit e88809e7288db9b7d33be8a67840e02a71327675
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Fri Sep 6 23:24:56 2019 -0700

    [CALCITE-3328] Immutable beans, powered by reflection
---
 .../org/apache/calcite/util/ImmutableBeans.java    | 386 ++++++++++++++++++
 .../org/apache/calcite/util/ImmutableBeanTest.java | 433 +++++++++++++++++++++
 2 files changed, 819 insertions(+)

diff --git a/core/src/main/java/org/apache/calcite/util/ImmutableBeans.java b/core/src/main/java/org/apache/calcite/util/ImmutableBeans.java
new file mode 100644
index 0000000..18954c9
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/util/ImmutableBeans.java
@@ -0,0 +1,386 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you 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
+ *
+ * http://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.
+ */
+package org.apache.calcite.util;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedMap;
+
+import java.lang.annotation.Annotation;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Proxy;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+
+/** Utilities for creating immutable beans. */
+public class ImmutableBeans {
+  private ImmutableBeans() {}
+
+  /** Creates an immutable bean that implements a given interface. */
+  public static <T> T create(Class<T> beanClass) {
+    if (!beanClass.isInterface()) {
+      throw new IllegalArgumentException("must be interface");
+    }
+    final ImmutableSortedMap.Builder<String, Class> propertyNameBuilder =
+        ImmutableSortedMap.naturalOrder();
+    final ImmutableMap.Builder<Method, Handler<T>> handlers =
+        ImmutableMap.builder();
+    final Set<String> requiredPropertyNames = new HashSet<>();
+
+    // First pass, add "get" methods and build a list of properties.
+    for (Method method : beanClass.getMethods()) {
+      if (!Modifier.isPublic(method.getModifiers())) {
+        continue;
+      }
+      final Property property = method.getAnnotation(Property.class);
+      if (property == null) {
+        continue;
+      }
+      final boolean hasNonnull = hasAnnotation(method, "javax.annotation.Nonnull");
+      final Mode mode;
+      final Object defaultValue = getDefault(method);
+      final String methodName = method.getName();
+      final String propertyName;
+      if (methodName.startsWith("get")) {
+        propertyName = methodName.substring("get".length());
+        mode = Mode.GET;
+      } else if (methodName.startsWith("is")) {
+        propertyName = methodName.substring("is".length());
+        mode = Mode.GET;
+      } else if (methodName.startsWith("with")) {
+        continue;
+      } else {
+        propertyName = methodName.substring(0, 1).toUpperCase(Locale.ROOT)
+            + methodName.substring(1);
+        mode = Mode.GET;
+      }
+      final Class<?> propertyType = method.getReturnType();
+      if (method.getParameterCount() > 0) {
+        throw new IllegalArgumentException("method '" + methodName
+            + "' has too many parameters");
+      }
+      final boolean required = property.required()
+          || propertyType.isPrimitive()
+          || hasNonnull;
+      if (required) {
+        requiredPropertyNames.add(propertyName);
+      }
+      propertyNameBuilder.put(propertyName, propertyType);
+      final Object defaultValue2 =
+          convertDefault(defaultValue, propertyName, propertyType);
+      handlers.put(method, (bean, args) -> {
+        switch (mode) {
+        case GET:
+          final Object v = bean.map.get(propertyName);
+          if (v != null) {
+            return v;
+          }
+          if (required && defaultValue == null) {
+            throw new IllegalArgumentException("property '" + propertyName
+                + "' is required and has no default value");
+          }
+          return defaultValue2;
+        default:
+          throw new AssertionError();
+        }
+      });
+    }
+
+    // Second pass, add "with" methods if they correspond to a property.
+    final ImmutableMap<String, Class> propertyNames =
+        propertyNameBuilder.build();
+    for (Method method : beanClass.getMethods()) {
+      if (!Modifier.isPublic(method.getModifiers())) {
+        continue;
+      }
+      final Mode mode;
+      final String propertyName;
+      final String methodName = method.getName();
+      if (methodName.startsWith("get")) {
+        continue;
+      } else if (methodName.startsWith("is")) {
+        continue;
+      } else if (methodName.startsWith("with")) {
+        propertyName = methodName.substring("with".length());
+        mode = Mode.WITH;
+      } else if (methodName.startsWith("set")) {
+        propertyName = methodName.substring("set".length());
+        mode = Mode.SET;
+      } else {
+        continue;
+      }
+      final Class propertyClass = propertyNames.get(propertyName);
+      if (propertyClass == null) {
+        throw new IllegalArgumentException("cannot find property '"
+            + propertyName + "' for method '" + methodName
+            + "'; maybe add a method 'get" + propertyName + "'?'");
+      }
+      switch (mode) {
+      case WITH:
+        if (method.getReturnType() != beanClass) {
+          throw new IllegalArgumentException("method '" + methodName
+              + "' should return the bean class '" + beanClass
+              + "', actually returns '" + method.getReturnType() + "'");
+        }
+        break;
+      case SET:
+        if (method.getReturnType() != void.class) {
+          throw new IllegalArgumentException("method '" + methodName
+              + "' should return void, actually returns '"
+              + method.getReturnType() + "'");
+        }
+      }
+      if (method.getParameterCount() != 1) {
+        throw new IllegalArgumentException("method '" + methodName
+            + "' should have one parameter, actually has "
+            + method.getParameterCount());
+      }
+      final Class propertyType = propertyNames.get(propertyName);
+      if (!method.getParameterTypes()[0].equals(propertyType)) {
+        throw new IllegalArgumentException("method '" + methodName
+            + "' should have parameter of type " + propertyType
+            + ", actually has " + method.getParameterTypes()[0]);
+      }
+      final boolean required = requiredPropertyNames.contains(propertyName);
+      handlers.put(method, (bean, args) -> {
+        switch (mode) {
+        case WITH:
+          final Object v = bean.map.get(propertyName);
+          final ImmutableMap.Builder<String, Object> mapBuilder;
+          if (v != null) {
+            if (v.equals(args[0])) {
+              return bean.asBean();
+            }
+            // the key already exists; painstakingly copy all entries
+            // except the one with this key
+            mapBuilder = ImmutableMap.builder();
+            bean.map.forEach((key, value) -> {
+              if (!key.equals(propertyName)) {
+                mapBuilder.put(key, value);
+              }
+            });
+          } else {
+            // the key does not exist; put the whole map into the builder
+            mapBuilder = ImmutableMap.<String, Object>builder()
+                .putAll(bean.map);
+          }
+          if (args[0] != null) {
+            mapBuilder.put(propertyName, args[0]);
+          } else {
+            if (required) {
+              throw new IllegalArgumentException("cannot set required "
+                  + "property '" + propertyName + "' to null");
+            }
+          }
+          final ImmutableMap<String, Object> map = mapBuilder.build();
+          return bean.withMap(map).asBean();
+        default:
+          throw new AssertionError();
+        }
+      });
+    }
+
+    handlers.put(getMethod(Object.class, "toString"),
+        (bean, args) -> new TreeMap<>(bean.map).toString());
+    handlers.put(getMethod(Object.class, "hashCode"),
+        (bean, args) -> new TreeMap<>(bean.map).hashCode());
+    handlers.put(getMethod(Object.class, "equals", Object.class),
+        (bean, args) -> bean == args[0]
+            // Use a little arg-swap trick because it's difficult to get inside
+            // a proxy
+            || beanClass.isInstance(args[0])
+            && args[0].equals(bean.map)
+            // Strictly, a bean should not equal a Map but it's convenient
+            || args[0] instanceof Map
+            && bean.map.equals(args[0]));
+    return makeBean(beanClass, handlers.build(), ImmutableMap.of());
+  }
+
+  /** Looks for an annotation by class name.
+   * Useful if you don't want to depend on the class
+   * (e.g. "javax.annotation.Nonnull") at compile time. */
+  private static boolean hasAnnotation(Method method, String className) {
+    for (Annotation annotation : method.getDeclaredAnnotations()) {
+      if (annotation.annotationType().getName().equals(className)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static Object getDefault(Method method) {
+    Object defaultValue = null;
+    final IntDefault intDefault = method.getAnnotation(IntDefault.class);
+    if (intDefault != null) {
+      defaultValue = intDefault.value();
+    }
+    final BooleanDefault booleanDefault =
+        method.getAnnotation(BooleanDefault.class);
+    if (booleanDefault != null) {
+      defaultValue = booleanDefault.value();
+    }
+    final StringDefault stringDefault =
+        method.getAnnotation(StringDefault.class);
+    if (stringDefault != null) {
+      defaultValue = stringDefault.value();
+    }
+    final EnumDefault enumDefault =
+        method.getAnnotation(EnumDefault.class);
+    if (enumDefault != null) {
+      defaultValue = enumDefault.value();
+    }
+    return defaultValue;
+  }
+
+  private static Object convertDefault(Object defaultValue, String propertyName,
+      Class propertyType) {
+    if (defaultValue == null || !propertyType.isEnum()) {
+      return defaultValue;
+    }
+    for (Object enumConstant : propertyType.getEnumConstants()) {
+      if (((Enum) enumConstant).name().equals(defaultValue)) {
+        return enumConstant;
+      }
+    }
+    throw new IllegalArgumentException("property '" + propertyName
+        + "' is an enum but its default value " + defaultValue
+        + " is not a valid enum constant");
+  }
+
+  private static Method getMethod(Class<Object> aClass,
+      String methodName, Class... parameterTypes) {
+    try {
+      return aClass.getMethod(methodName, parameterTypes);
+    } catch (NoSuchMethodException e) {
+      throw new AssertionError();
+    }
+  }
+
+  private static <T> T makeBean(Class<T> beanClass,
+      ImmutableMap<Method, Handler<T>> handlers,
+      ImmutableMap<String, Object> map) {
+    return new BeanImpl<>(beanClass, handlers, map).asBean();
+  }
+
+  /** Is the method reading or writing? */
+  private enum Mode {
+    GET, SET, WITH
+  }
+
+  /** Handler for a particular method call; called with "this" and arguments.
+   *
+   * @param <T> Bean type */
+  private interface Handler<T> {
+    Object apply(BeanImpl<T> bean, Object[] args);
+  }
+
+  /** Property of a bean. Apply this annotation to the "get" method. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface Property {
+    /** Whether the property is required.
+     *
+     * <p>Properties of type {@code int} and {@code boolean} are always
+     * required.
+     *
+     * <p>If a property is required, it cannot be set to null.
+     * If it has no default value, calling "get" will give a runtime exception.
+     */
+    boolean required() default false;
+  }
+
+  /** Default value of an int property. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface IntDefault {
+    int value();
+  }
+
+  /** Default value of a boolean property of a bean. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface BooleanDefault {
+    boolean value();
+  }
+
+  /** Default value of a String property of a bean. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface StringDefault {
+    String value();
+  }
+
+  /** Default value of an enum property of a bean. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface EnumDefault {
+    String value();
+  }
+
+  /** Default value of a String or enum property of a bean that is null. */
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.METHOD)
+  public @interface NullDefault {
+  }
+
+  /** Implementation of an instance of a bean; stores property
+   * values in a map, and also implements {@code InvocationHandler}
+   * so that it can retrieve calls from a reflective proxy.
+   *
+   * @param <T> Bean type */
+  private static class BeanImpl<T> implements InvocationHandler {
+    private final ImmutableMap<Method, Handler<T>> handlers;
+    private final ImmutableMap<String, Object> map;
+    private final Class<T> beanClass;
+
+    BeanImpl(Class<T> beanClass, ImmutableMap<Method, Handler<T>> handlers,
+        ImmutableMap<String, Object> map) {
+      this.beanClass = beanClass;
+      this.handlers = handlers;
+      this.map = map;
+    }
+
+    public Object invoke(Object proxy, Method method, Object[] args) {
+      final Handler handler = handlers.get(method);
+      if (handler == null) {
+        throw new IllegalArgumentException("no handler for method " + method);
+      }
+      return handler.apply(this, args);
+    }
+
+    /** Returns a copy of this bean that has a different map. */
+    BeanImpl<T> withMap(ImmutableMap<String, Object> map) {
+      return new BeanImpl<T>(beanClass, handlers, map);
+    }
+
+    /** Wraps this handler in a proxy that implements the required
+     * interface. */
+    T asBean() {
+      return beanClass.cast(
+          Proxy.newProxyInstance(beanClass.getClassLoader(),
+              new Class[] {beanClass}, this));
+    }
+  }
+}
diff --git a/core/src/test/java/org/apache/calcite/util/ImmutableBeanTest.java b/core/src/test/java/org/apache/calcite/util/ImmutableBeanTest.java
new file mode 100644
index 0000000..866a1ef
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/util/ImmutableBeanTest.java
@@ -0,0 +1,433 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you 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
+ *
+ * http://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.
+ */
+package org.apache.calcite.util;
+
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+import java.util.TreeMap;
+import javax.annotation.Nonnull;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.core.IsNull.nullValue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+/** Unit test for {@link ImmutableBeans}. */
+public class ImmutableBeanTest {
+
+  @Test public void testSimple() {
+    final MyBean b = ImmutableBeans.create(MyBean.class);
+    assertThat(b.withFoo(1).getFoo(), is(1));
+    assertThat(b.withBar(false).isBar(), is(false));
+    assertThat(b.withBaz("a").getBaz(), is("a"));
+    assertThat(b.withBaz("a").withBaz("a").getBaz(), is("a"));
+
+    // Calling "with" on b2 does not change the "foo" property
+    final MyBean b2 = b.withFoo(2);
+    final MyBean b3 = b2.withFoo(3);
+    assertThat(b3.getFoo(), is(3));
+    assertThat(b2.getFoo(), is(2));
+
+    final MyBean b4 = b2.withFoo(3).withBar(true).withBaz("xyz");
+    final Map<String, Object> map = new TreeMap<>();
+    map.put("Foo", b4.getFoo());
+    map.put("Bar", b4.isBar());
+    map.put("Baz", b4.getBaz());
+    assertThat(b4.toString(), is(map.toString()));
+    assertThat(b4.hashCode(), is(map.hashCode()));
+    final MyBean b5 = b2.withFoo(3).withBar(true).withBaz("xyz");
+    assertThat(b4.equals(b5), is(true));
+    assertThat(b4.equals(b), is(false));
+    assertThat(b4.equals(b2), is(false));
+    assertThat(b4.equals(b3), is(false));
+  }
+
+  @Test public void testDefault() {
+    final Bean2 b = ImmutableBeans.create(Bean2.class);
+
+    // int, no default
+    try {
+      final int v = b.getIntSansDefault();
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("property 'IntSansDefault' is required and has no default value"));
+    }
+    assertThat(b.withIntSansDefault(4).getIntSansDefault(), is(4));
+
+    // int, with default
+    assertThat(b.getIntWithDefault(), is(1));
+    assertThat(b.withIntWithDefault(10).getIntWithDefault(), is(10));
+    assertThat(b.withIntWithDefault(1).getIntWithDefault(), is(1));
+
+    // boolean, no default
+    try {
+      final boolean v = b.isBooleanSansDefault();
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("property 'BooleanSansDefault' is required and has no default "
+              + "value"));
+    }
+    assertThat(b.withBooleanSansDefault(false).isBooleanSansDefault(),
+        is(false));
+
+    // boolean, with default
+    assertThat(b.isBooleanWithDefault(), is(true));
+    assertThat(b.withBooleanWithDefault(false).isBooleanWithDefault(),
+        is(false));
+    assertThat(b.withBooleanWithDefault(true).isBooleanWithDefault(),
+        is(true));
+
+    // string, no default
+    try {
+      final String v = b.getStringSansDefault();
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("property 'StringSansDefault' is required and has no default "
+              + "value"));
+    }
+    assertThat(b.withStringSansDefault("a").getStringSansDefault(), is("a"));
+
+    // string, no default
+    try {
+      final String v = b.getNonnullString();
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("property 'NonnullString' is required and has no default value"));
+    }
+    assertThat(b.withNonnullString("a").getNonnullString(), is("a"));
+
+    // string, with default
+    assertThat(b.getStringWithDefault(), is("abc"));
+    assertThat(b.withStringWithDefault("").getStringWithDefault(), is(""));
+    assertThat(b.withStringWithDefault("x").getStringWithDefault(), is("x"));
+    assertThat(b.withStringWithDefault("abc").getStringWithDefault(),
+        is("abc"));
+    try {
+      final Bean2 v = b.withStringWithDefault(null);
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("cannot set required property 'StringWithDefault' to null"));
+    }
+
+    // string, optional
+    assertThat(b.getOptionalString(), nullValue());
+    assertThat(b.withOptionalString("").getOptionalString(), is(""));
+    assertThat(b.withOptionalString("x").getOptionalString(), is("x"));
+    assertThat(b.withOptionalString("abc").getOptionalString(), is("abc"));
+    assertThat(b.withOptionalString(null).getOptionalString(), nullValue());
+
+    // string, optional
+    assertThat(b.getStringWithNullDefault(), nullValue());
+    assertThat(b.withStringWithNullDefault("").getStringWithNullDefault(),
+        is(""));
+    assertThat(b.withStringWithNullDefault("x").getStringWithNullDefault(),
+        is("x"));
+    assertThat(b.withStringWithNullDefault("abc").getStringWithNullDefault(),
+        is("abc"));
+    assertThat(b.withStringWithNullDefault(null).getStringWithNullDefault(),
+        nullValue());
+
+    // enum, with default
+    assertThat(b.getColorWithDefault(), is(Color.RED));
+    assertThat(b.withColorWithDefault(Color.GREEN).getColorWithDefault(),
+        is(Color.GREEN));
+    assertThat(b.withColorWithDefault(Color.RED).getColorWithDefault(),
+        is(Color.RED));
+    try {
+      final Bean2 v = b.withColorWithDefault(null);
+      throw new AssertionError("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(),
+          is("cannot set required property 'ColorWithDefault' to null"));
+    }
+
+    // color, optional
+    assertThat(b.getColorOptional(), nullValue());
+    assertThat(b.withColorOptional(Color.RED).getColorOptional(),
+        is(Color.RED));
+    assertThat(b.withColorOptional(Color.RED).withColorOptional(null)
+        .getColorOptional(), nullValue());
+    assertThat(b.withColorOptional(null).getColorOptional(), nullValue());
+    assertThat(b.withColorOptional(Color.RED).withColorOptional(Color.GREEN)
+        .getColorOptional(), is(Color.GREEN));
+
+    // color, optional with null default
+    assertThat(b.getColorWithNullDefault(), nullValue());
+    assertThat(b.withColorWithNullDefault(null).getColorWithNullDefault(),
+        nullValue());
+    assertThat(b.withColorWithNullDefault(Color.RED).getColorWithNullDefault(),
+        is(Color.RED));
+    assertThat(b.withColorWithNullDefault(Color.RED)
+        .withColorWithNullDefault(null).getColorWithNullDefault(), nullValue());
+    assertThat(b.withColorWithNullDefault(Color.RED)
+        .withColorWithNullDefault(Color.GREEN).getColorWithNullDefault(),
+        is(Color.GREEN));
+
+    // Default values do not appear in toString().
+    // (Maybe they should... but then they'd be initial values?)
+    assertThat(b.toString(), is("{}"));
+
+    // Beans with values explicitly set are not equal to
+    // beans with the same values via defaults.
+    // (I could be persuaded that this is the wrong behavior.)
+    assertThat(b.equals(b.withIntWithDefault(1)), is(false));
+    assertThat(b.withIntWithDefault(1).equals(b.withIntWithDefault(1)),
+        is(true));
+    assertThat(b.withIntWithDefault(1).equals(b.withIntWithDefault(2)),
+        is(false));
+  }
+
+  private void check(Class<?> beanClass, Matcher<String> matcher) {
+    try {
+      final Object v = ImmutableBeans.create(beanClass);
+      fail("expected error, got " + v);
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage(), matcher);
+    }
+  }
+
+  @Test public void testValidate() {
+    check(BeanWhoseDefaultIsBadEnumValue.class,
+        is("property 'Color' is an enum but its default value YELLOW is not a "
+            + "valid enum constant"));
+    check(BeanWhoseWithMethodHasBadReturnType.class,
+        is("method 'withFoo' should return the bean class 'interface "
+            + "org.apache.calcite.util.ImmutableBeanTest$"
+            + "BeanWhoseWithMethodHasBadReturnType', actually returns "
+            + "'interface org.apache.calcite.util.ImmutableBeanTest$MyBean'"));
+    check(BeanWhoseWithMethodDoesNotMatchProperty.class,
+        is("method 'withFoo' should return the bean class 'interface "
+            + "org.apache.calcite.util.ImmutableBeanTest$"
+            + "BeanWhoseWithMethodDoesNotMatchProperty', actually returns "
+            + "'interface org.apache.calcite.util.ImmutableBeanTest$MyBean'"));
+    check(BeanWhoseWithMethodHasArgOfWrongType.class,
+        is("method 'withFoo' should return the bean class 'interface "
+            + "org.apache.calcite.util.ImmutableBeanTest$"
+            + "BeanWhoseWithMethodHasArgOfWrongType', actually returns "
+            + "'interface org.apache.calcite.util.ImmutableBeanTest$"
+            + "BeanWhoseWithMethodHasTooManyArgs'"));
+    check(BeanWhoseWithMethodHasTooManyArgs.class,
+        is("method 'withFoo' should have one parameter, actually has 2"));
+    check(BeanWhoseWithMethodHasTooFewArgs.class,
+        is("method 'withFoo' should have one parameter, actually has 0"));
+    check(BeanWhoseSetMethodHasBadReturnType.class,
+        is("method 'setFoo' should return void, actually returns "
+            + "'interface org.apache.calcite.util.ImmutableBeanTest$MyBean'"));
+    check(BeanWhoseGetMethodHasTooManyArgs.class,
+        is("method 'getFoo' has too many parameters"));
+    check(BeanWhoseSetMethodDoesNotMatchProperty.class,
+        is("cannot find property 'Foo' for method 'setFoo'; maybe add a method "
+            + "'getFoo'?'"));
+    check(BeanWhoseSetMethodHasArgOfWrongType.class,
+        is("method 'setFoo' should have parameter of type int, actually has "
+            + "float"));
+    check(BeanWhoseSetMethodHasTooManyArgs.class,
+        is("method 'setFoo' should have one parameter, actually has 2"));
+    check(BeanWhoseSetMethodHasTooFewArgs.class,
+        is("method 'setFoo' should have one parameter, actually has 0"));
+  }
+
+  /** Bean whose default value is not a valid value for the enum;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseDefaultIsBadEnumValue {
+    @ImmutableBeans.Property
+    @ImmutableBeans.EnumDefault("YELLOW")
+    Color getColor();
+    BeanWhoseDefaultIsBadEnumValue withColor(Color color);
+  }
+
+  /** Bean that has a 'with' method that has a bad return type;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseWithMethodHasBadReturnType {
+    @ImmutableBeans.Property int getFoo();
+    MyBean withFoo(int x);
+  }
+
+  /** Bean that has a 'with' method that does not correspond to a property
+   * (declared using a {@link ImmutableBeans.Property} annotation on a
+   * 'get' method;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseWithMethodDoesNotMatchProperty {
+    @ImmutableBeans.Property int getFoo();
+    MyBean withFoo(int x);
+  }
+
+  /** Bean that has a 'with' method whose argument type is not the same as the
+   * type of the property (the return type of a 'get{PropertyName}' method);
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseWithMethodHasArgOfWrongType {
+    @ImmutableBeans.Property int getFoo();
+    BeanWhoseWithMethodHasTooManyArgs withFoo(float x);
+  }
+
+  /** Bean that has a 'with' method that has too many arguments;
+   * it should have just one;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseWithMethodHasTooManyArgs {
+    @ImmutableBeans.Property int getFoo();
+    BeanWhoseWithMethodHasTooManyArgs withFoo(int x, int y);
+  }
+
+  /** Bean that has a 'with' method that has too few arguments;
+   * it should have just one;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseWithMethodHasTooFewArgs {
+    @ImmutableBeans.Property int getFoo();
+    BeanWhoseWithMethodHasTooFewArgs withFoo();
+  }
+
+  /** Bean that has a 'set' method that has a bad return type;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseSetMethodHasBadReturnType {
+    @ImmutableBeans.Property int getFoo();
+    MyBean setFoo(int x);
+  }
+
+  /** Bean that has a 'get' method that has one arg, whereas 'get' must have no
+   * args;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseGetMethodHasTooManyArgs {
+    @ImmutableBeans.Property int getFoo(int x);
+    void setFoo(int x);
+  }
+
+  /** Bean that has a 'set' method that does not correspond to a property
+   * (declared using a {@link ImmutableBeans.Property} annotation on a
+   * 'get' method;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseSetMethodDoesNotMatchProperty {
+    @ImmutableBeans.Property int getBar();
+    void setFoo(int x);
+  }
+
+  /** Bean that has a 'set' method whose argument type is not the same as the
+   * type of the property (the return type of a 'get{PropertyName}' method);
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseSetMethodHasArgOfWrongType {
+    @ImmutableBeans.Property int getFoo();
+    void setFoo(float x);
+  }
+
+  /** Bean that has a 'set' method that has too many arguments;
+   * it should have just one;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseSetMethodHasTooManyArgs {
+    @ImmutableBeans.Property int getFoo();
+    void setFoo(int x, int y);
+  }
+
+  /** Bean that has a 'set' method that has too few arguments;
+   * it should have just one;
+   * used in {@link #testValidate()}. */
+  interface BeanWhoseSetMethodHasTooFewArgs {
+    @ImmutableBeans.Property int getFoo();
+    void setFoo();
+  }
+
+  // ditto setXxx
+
+    // TODO it is an error to declare an int property to be not required
+    // TODO it is an error to declare an boolean property to be not required
+
+  /** A simple bean with properties of various types, no defaults. */
+  interface MyBean {
+    @ImmutableBeans.Property
+    int getFoo();
+    MyBean withFoo(int x);
+
+    @ImmutableBeans.Property
+    boolean isBar();
+    MyBean withBar(boolean x);
+
+    @ImmutableBeans.Property
+    String getBaz();
+    MyBean withBaz(String s);
+  }
+
+  /** A bean class with just about every combination of default values
+   * missing and present, and required or not. */
+  interface Bean2 {
+    @ImmutableBeans.Property
+    @ImmutableBeans.IntDefault(1)
+    int getIntWithDefault();
+    Bean2 withIntWithDefault(int x);
+
+    @ImmutableBeans.Property
+    int getIntSansDefault();
+    Bean2 withIntSansDefault(int x);
+
+    @ImmutableBeans.Property
+    @ImmutableBeans.BooleanDefault(true)
+    boolean isBooleanWithDefault();
+    Bean2 withBooleanWithDefault(boolean x);
+
+    @ImmutableBeans.Property
+    boolean isBooleanSansDefault();
+    Bean2 withBooleanSansDefault(boolean x);
+
+    @ImmutableBeans.Property(required = true)
+    String getStringSansDefault();
+    Bean2 withStringSansDefault(String x);
+
+    @ImmutableBeans.Property
+    String getOptionalString();
+    Bean2 withOptionalString(String s);
+
+    /** Property is required because it has 'Nonnull' annotation. */
+    @ImmutableBeans.Property
+    @Nonnull String getNonnullString();
+    Bean2 withNonnullString(String s);
+
+    @ImmutableBeans.Property
+    @ImmutableBeans.StringDefault("abc")
+    @Nonnull String getStringWithDefault();
+    Bean2 withStringWithDefault(String s);
+
+    @ImmutableBeans.Property
+    @ImmutableBeans.NullDefault
+    String getStringWithNullDefault();
+    Bean2 withStringWithNullDefault(String s);
+
+    @ImmutableBeans.Property
+    @ImmutableBeans.EnumDefault("RED")
+    @Nonnull Color getColorWithDefault();
+    Bean2 withColorWithDefault(Color color);
+
+    @ImmutableBeans.Property
+    @ImmutableBeans.NullDefault
+    Color getColorWithNullDefault();
+    Bean2 withColorWithNullDefault(Color color);
+
+    @ImmutableBeans.Property()
+    Color getColorOptional();
+    Bean2 withColorOptional(Color color);
+  }
+
+  /** Red, blue, green. */
+  enum Color {
+    RED,
+    BLUE,
+    GREEN
+  }
+}


[calcite] 07/07: [CALCITE-3526] SqlPrettyWriter should have options to fold/chop long lines, print leading commas

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit c416c31fc376868bdd672afd84ec06dc75d56575
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Oct 28 13:02:01 2019 -0700

    [CALCITE-3526] SqlPrettyWriter should have options to fold/chop long lines, print leading commas
    
    Move SqlPrettyWriter.Config to top-level class SqlWriterConfig, and
    make it an interface implemented via ImmutableBeans; fixes
      [CALCITE-1585] SqlPrettyWriter doesn't respect alwaysUseParentheses
    because config is now held in an immutable object rather than the
    SqlPrettyWriter.
    
    Add option SqlWriterConfig.withLeadingComma, to generate
    
      SELECT x
          , y
      FROM t
    
    rather than
    
      SELECT x,
          y
      FROM t
    
    Add option SqlWriterConfig.fromLength, which controls when lines are
    considered long enough to chop or fold. Unlike lineLength(), which is
    absolute, it is relative to the current sub-query.
    
    Refactor: add SqlNodeList.SINGLETON_STAR.
    
    Add lots of options including selectFolding(), fromFolding(),
    clauseEndsList().
    
    Extend SqlParserTest to test randomly-configured SqlPrettyWriter.
---
 .../org/apache/calcite/test/BabelQuidemTest.java   |    4 +-
 babel/src/test/resources/sql/select.iq             |    2 +-
 .../calcite/adapter/cassandra/CassandraSchema.java |   13 +-
 .../org/apache/calcite/adapter/jdbc/JdbcTable.java |   12 +-
 .../calcite/rel/rel2sql/RelToSqlConverter.java     |    3 +-
 .../java/org/apache/calcite/sql/SqlAsOperator.java |    2 +-
 .../main/java/org/apache/calcite/sql/SqlJoin.java  |   20 +-
 .../org/apache/calcite/sql/SqlMatchRecognize.java  |    6 +-
 .../main/java/org/apache/calcite/sql/SqlNode.java  |   44 +-
 .../java/org/apache/calcite/sql/SqlNodeList.java   |   57 +-
 .../java/org/apache/calcite/sql/SqlOperator.java   |   36 +-
 .../java/org/apache/calcite/sql/SqlOrderBy.java    |    6 +-
 .../org/apache/calcite/sql/SqlSelectOperator.java  |   65 +-
 .../apache/calcite/sql/SqlWithinGroupOperator.java |    2 +-
 .../java/org/apache/calcite/sql/SqlWriter.java     |   55 +-
 .../org/apache/calcite/sql/SqlWriterConfig.java    |  434 +++++++
 .../apache/calcite/sql/fun/SqlCaseOperator.java    |    5 +-
 .../calcite/sql/fun/SqlStdOperatorTable.java       |    1 +
 .../apache/calcite/sql/pretty/SqlPrettyWriter.java | 1231 ++++++++++++--------
 .../apache/calcite/sql/type/IntervalSqlType.java   |   11 +-
 .../apache/calcite/sql/parser/SqlParserTest.java   |  168 ++-
 .../calcite/sql/test/SqlOperatorBaseTest.java      |    4 +-
 .../calcite/sql/test/SqlPrettyWriterTest.java      |  262 ++++-
 .../org/apache/calcite/test/JdbcAdapterTest.java   |    2 +-
 .../calcite/sql/test/SqlPrettyWriterTest.xml       |  337 +++++-
 .../org/apache/calcite/piglet/PigConverter.java    |   13 +-
 .../java/org/apache/calcite/test/PigRelOpTest.java |  122 +-
 .../org/apache/calcite/sql/ddl/SqlDdlNodes.java    |   22 +-
 28 files changed, 2106 insertions(+), 833 deletions(-)

diff --git a/babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java b/babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java
index bc82d34..51477db 100644
--- a/babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java
+++ b/babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java
@@ -23,7 +23,6 @@ import org.apache.calcite.plan.Contexts;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlWriter;
-import org.apache.calcite.sql.dialect.CalciteSqlDialect;
 import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
@@ -152,8 +151,7 @@ public class BabelQuidemTest extends QuidemTest {
         final Planner planner = Frameworks.getPlanner(config.build());
         final SqlNode node = planner.parse(sqlCommand.sql);
         final SqlNode validateNode = planner.validate(node);
-        final SqlWriter sqlWriter =
-            new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+        final SqlWriter sqlWriter = new SqlPrettyWriter();
         validateNode.unparse(sqlWriter, 0, 0);
         x.echo(ImmutableList.of(sqlWriter.toSqlString().getSql()));
       } else {
diff --git a/babel/src/test/resources/sql/select.iq b/babel/src/test/resources/sql/select.iq
index 4e9f822..07399eb 100755
--- a/babel/src/test/resources/sql/select.iq
+++ b/babel/src/test/resources/sql/select.iq
@@ -25,7 +25,7 @@ ORDER BY emp.deptno;
 
 SELECT "EMP"."ENAME"
 FROM "scott"."EMP" AS "EMP",
-        "scott"."DEPT" AS "DEPT"
+    "scott"."DEPT" AS "DEPT"
 ORDER BY "EMP"."DEPTNO"
 !explain-validated-on all
 
diff --git a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java
index fd6819c..85c4c6d 100644
--- a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java
+++ b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java
@@ -30,7 +30,7 @@ import org.apache.calcite.schema.impl.AbstractSchema;
 import org.apache.calcite.schema.impl.MaterializedViewTable;
 import org.apache.calcite.sql.SqlSelect;
 import org.apache.calcite.sql.SqlWriter;
-import org.apache.calcite.sql.dialect.CalciteSqlDialect;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
@@ -54,8 +54,6 @@ import com.google.common.collect.ImmutableMap;
 
 import org.slf4j.Logger;
 
-import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.List;
@@ -290,11 +288,12 @@ public class CassandraSchema extends AbstractSchema {
         continue;
       }
 
-      StringWriter stringWriter = new StringWriter(query.length());
-      PrintWriter printWriter = new PrintWriter(stringWriter);
-      SqlWriter writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT, true, printWriter);
+      final StringBuilder buf = new StringBuilder(query.length());
+      final SqlWriterConfig config = SqlPrettyWriter.config()
+          .withAlwaysUseParentheses(true);
+      final SqlWriter writer = new SqlPrettyWriter(config, buf);
       parsedQuery.unparse(writer, 0, 0);
-      query = stringWriter.toString();
+      query = buf.toString();
 
       // Add the view for this query
       String viewName = "$" + getTableNames().size();
diff --git a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcTable.java b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcTable.java
index b1f32ad..3a8eff5 100644
--- a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcTable.java
@@ -47,6 +47,7 @@ import org.apache.calcite.schema.impl.AbstractTableQueryable;
 import org.apache.calcite.sql.SqlIdentifier;
 import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlSelect;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
 import org.apache.calcite.sql.util.SqlString;
@@ -58,7 +59,6 @@ import com.google.common.collect.Lists;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 
@@ -141,14 +141,14 @@ public class JdbcTable extends AbstractQueryableTable
   }
 
   SqlString generateSql() {
-    final SqlNodeList selectList =
-        new SqlNodeList(
-            Collections.singletonList(SqlIdentifier.star(SqlParserPos.ZERO)),
-            SqlParserPos.ZERO);
+    final SqlNodeList selectList = SqlNodeList.SINGLETON_STAR;
     SqlSelect node =
         new SqlSelect(SqlParserPos.ZERO, SqlNodeList.EMPTY, selectList,
             tableName(), null, null, null, null, null, null, null, null);
-    final SqlPrettyWriter writer = new SqlPrettyWriter(jdbcSchema.dialect);
+    final SqlWriterConfig config = SqlPrettyWriter.config()
+        .withAlwaysUseParentheses(true)
+        .withDialect(jdbcSchema.dialect);
+    final SqlPrettyWriter writer = new SqlPrettyWriter(config);
     node.unparse(writer, 0, 0);
     return writer.toSqlString();
   }
diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
index e41b1ce..d499a1c 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
@@ -521,8 +521,7 @@ public class RelToSqlConverter extends SqlImplementor
 
           // Wrap "SELECT 1 AS x"
           // as "SELECT * FROM (SELECT 1 AS x) AS t WHERE false"
-          query = new SqlSelect(POS, null,
-              new SqlNodeList(ImmutableList.of(SqlIdentifier.star(POS)), POS),
+          query = new SqlSelect(POS, null, SqlNodeList.SINGLETON_STAR,
               as(query, "t"), createAlwaysFalseCondition(), null, null,
               null, null, null, null, null);
         } else {
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java
index 5e2b7cf..e90340d 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java
@@ -72,7 +72,7 @@ public class SqlAsOperator extends SqlSpecialOperator {
     assert call.operandCount() >= 2;
     final SqlWriter.Frame frame =
         writer.startList(
-            SqlWriter.FrameTypeEnum.SIMPLE);
+            SqlWriter.FrameTypeEnum.AS);
     call.operand(0).unparse(writer, leftPrec, getLeftPrec());
     final boolean needsSpace = true;
     writer.setNeedWhitespace(needsSpace);
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJoin.java b/core/src/main/java/org/apache/calcite/sql/SqlJoin.java
index 1977aa6..21663e8 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlJoin.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlJoin.java
@@ -196,37 +196,32 @@ public class SqlJoin extends SqlCall {
         int rightPrec) {
       final SqlJoin join = (SqlJoin) call;
 
-      final SqlWriter.Frame joinFrame =
-          writer.startList(SqlWriter.FrameTypeEnum.JOIN);
       join.left.unparse(
           writer,
           leftPrec,
           getLeftPrec());
-      String natural = "";
-      if (join.isNatural()) {
-        natural = "NATURAL ";
-      }
       switch (join.getJoinType()) {
       case COMMA:
         writer.sep(",", true);
         break;
       case CROSS:
-        writer.sep(natural + "CROSS JOIN");
+        writer.sep(join.isNatural() ? "NATURAL CROSS JOIN" : "CROSS JOIN");
         break;
       case FULL:
-        writer.sep(natural + "FULL JOIN");
+        writer.sep(join.isNatural() ? "NATURAL FULL JOIN" : "FULL JOIN");
         break;
       case INNER:
-        writer.sep(natural + "INNER JOIN");
+        writer.sep(join.isNatural() ? "NATURAL INNER JOIN" : "INNER JOIN");
         break;
       case LEFT:
-        writer.sep(natural + "LEFT JOIN");
+        writer.sep(join.isNatural() ? "NATURAL LEFT JOIN" : "LEFT JOIN");
         break;
       case LEFT_SEMI_JOIN:
-        writer.sep(natural + "LEFT SEMI JOIN");
+        writer.sep(join.isNatural() ? "NATURAL LEFT SEMI JOIN"
+            : "LEFT SEMI JOIN");
         break;
       case RIGHT:
-        writer.sep(natural + "RIGHT JOIN");
+        writer.sep(join.isNatural() ? "NATURAL RIGHT JOIN" : "RIGHT JOIN");
         break;
       default:
         throw Util.unexpected(join.getJoinType());
@@ -254,7 +249,6 @@ public class SqlJoin extends SqlCall {
           throw Util.unexpected(join.getConditionType());
         }
       }
-      writer.endList(joinFrame);
     }
   }
 }
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlMatchRecognize.java b/core/src/main/java/org/apache/calcite/sql/SqlMatchRecognize.java
index ecbc106..c4c156b 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlMatchRecognize.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlMatchRecognize.java
@@ -336,10 +336,8 @@ public class SqlMatchRecognize extends SqlCall {
       if (pattern.orderList != null && pattern.orderList.size() > 0) {
         writer.newlineAndIndent();
         writer.sep("ORDER BY");
-        final SqlWriter.Frame orderFrame =
-            writer.startList(SqlWriter.FrameTypeEnum.ORDER_BY_LIST);
-        unparseListClause(writer, pattern.orderList);
-        writer.endList(orderFrame);
+        writer.list(SqlWriter.FrameTypeEnum.ORDER_BY_LIST, SqlWriter.COMMA,
+            pattern.orderList);
       }
 
       if (pattern.measureList != null && pattern.measureList.size() > 0) {
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlNode.java b/core/src/main/java/org/apache/calcite/sql/SqlNode.java
index ca3434c..9947f78 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlNode.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlNode.java
@@ -32,6 +32,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.UnaryOperator;
 import javax.annotation.Nonnull;
 
 /**
@@ -122,7 +123,33 @@ public abstract class SqlNode implements Cloneable {
   }
 
   public String toString() {
-    return toSqlString(null).getSql();
+    return toSqlString(c -> c.withDialect(AnsiSqlDialect.DEFAULT)
+        .withAlwaysUseParentheses(false)
+        .withSelectListItemsOnSeparateLines(false)
+        .withUpdateSetListNewline(false)
+        .withIndentation(0)).getSql();
+  }
+
+  /**
+   * Returns the SQL text of the tree of which this <code>SqlNode</code> is
+   * the root.
+   *
+   * <p>Typical return values are:
+   *
+   * <ul>
+   * <li>'It''s a bird!'
+   * <li>NULL
+   * <li>12.3
+   * <li>DATE '1969-04-29'
+   * </ul>
+   *
+   * @param transform   Transform that sets desired writer configuration
+   */
+  public SqlString toSqlString(UnaryOperator<SqlWriterConfig> transform) {
+    final SqlWriterConfig config = transform.apply(SqlPrettyWriter.config());
+    SqlPrettyWriter writer = new SqlPrettyWriter(config);
+    unparse(writer, 0, 0);
+    return writer.toSqlString();
   }
 
   /**
@@ -143,15 +170,12 @@ public abstract class SqlNode implements Cloneable {
    *                    useful for parse test, but false by default
    */
   public SqlString toSqlString(SqlDialect dialect, boolean forceParens) {
-    if (dialect == null) {
-      dialect = AnsiSqlDialect.DEFAULT;
-    }
-    SqlPrettyWriter writer = new SqlPrettyWriter(dialect);
-    writer.setAlwaysUseParentheses(forceParens);
-    writer.setSelectListItemsOnSeparateLines(false);
-    writer.setIndentation(0);
-    unparse(writer, 0, 0);
-    return writer.toSqlString();
+    return toSqlString(c ->
+        c.withDialect(Util.first(dialect, AnsiSqlDialect.DEFAULT))
+            .withAlwaysUseParentheses(forceParens)
+            .withSelectListItemsOnSeparateLines(false)
+            .withUpdateSetListNewline(false)
+            .withIndentation(0));
   }
 
   public SqlString toSqlString(SqlDialect dialect) {
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlNodeList.java b/core/src/main/java/org/apache/calcite/sql/SqlNodeList.java
index 426869d..d1a5329 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlNodeList.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlNodeList.java
@@ -16,13 +16,14 @@
  */
 package org.apache.calcite.sql;
 
-import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.util.SqlVisitor;
 import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql.validate.SqlValidatorScope;
 import org.apache.calcite.util.Litmus;
 
+import com.google.common.collect.ImmutableList;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -45,6 +46,19 @@ public class SqlNodeList extends SqlNode implements Iterable<SqlNode> {
         }
       };
 
+  /**
+   * A SqlNodeList that has a single element that is an empty list.
+   */
+  public static final SqlNodeList SINGLETON_EMPTY =
+      new SqlNodeList(ImmutableList.of(EMPTY), SqlParserPos.ZERO);
+
+  /**
+   * A SqlNodeList that has a single element that is a star identifier.
+   */
+  public static final SqlNodeList SINGLETON_STAR =
+      new SqlNodeList(ImmutableList.of(SqlIdentifier.star(SqlParserPos.ZERO)),
+          SqlParserPos.ZERO);
+
   //~ Instance fields --------------------------------------------------------
 
   private final List<SqlNode> list;
@@ -105,42 +119,21 @@ public class SqlNodeList extends SqlNode implements Iterable<SqlNode> {
       SqlWriter writer,
       int leftPrec,
       int rightPrec) {
-    final SqlWriter.Frame frame =
-        ((leftPrec > 0) || (rightPrec > 0)) ? writer.startList("(", ")")
-            : writer.startList("", "");
-    commaList(writer);
-    writer.endList(frame);
+    final SqlWriter.FrameTypeEnum frameType =
+        (leftPrec > 0 || rightPrec > 0)
+            ? SqlWriter.FrameTypeEnum.PARENTHESES
+            : SqlWriter.FrameTypeEnum.SIMPLE;
+    writer.list(frameType, SqlWriter.COMMA, this);
   }
 
+  @Deprecated // to be removed before 2.0
   void commaList(SqlWriter writer) {
-    // The precedence of the comma operator if low but not zero. For
-    // instance, this ensures parentheses in
-    //    select x, (select * from foo order by z), y from t
-    for (SqlNode node : list) {
-      writer.sep(",");
-      node.unparse(writer, 2, 3);
-    }
+    unparse(writer, 0, 0);
   }
 
-  void andOrList(SqlWriter writer, SqlKind sepKind) {
-    SqlBinaryOperator sepOp =
-        sepKind == SqlKind.AND
-            ? SqlStdOperatorTable.AND
-            : SqlStdOperatorTable.OR;
-    for (int i = 0; i < list.size(); i++) {
-      SqlNode node = list.get(i);
-      writer.sep(sepKind.name(), false);
-
-      // The precedence pulling on the LHS of a node is the
-      // right-precedence of the separator operator, except at the start
-      // of the list; similarly for the RHS of a node. If the operator
-      // has left precedence 4 and right precedence 5, the precedences
-      // in a 3-node list will look as follows:
-      //   0 <- node1 -> 4  5 <- node2 -> 4  5 <- node3 -> 0
-      int lprec = (i == 0) ? 0 : sepOp.getRightPrec();
-      int rprec = (i == (list.size() - 1)) ? 0 : sepOp.getLeftPrec();
-      node.unparse(writer, lprec, rprec);
-    }
+  @Deprecated // to be removed before 2.0
+  void andOrList(SqlWriter writer, SqlBinaryOperator sepOp) {
+    writer.list(SqlWriter.FrameTypeEnum.WHERE_LIST, sepOp, this);
   }
 
   public void validate(SqlValidator validator, SqlValidatorScope scope) {
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
index 1eb330d..3c96f02 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
@@ -19,6 +19,7 @@ package org.apache.calcite.sql;
 import org.apache.calcite.linq4j.Ord;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.type.SqlOperandTypeChecker;
 import org.apache.calcite.sql.type.SqlOperandTypeInference;
@@ -332,25 +333,40 @@ public abstract class SqlOperator {
     getSyntax().unparse(writer, this, call, leftPrec, rightPrec);
   }
 
-  // REVIEW jvs 9-June-2006: See http://issues.eigenbase.org/browse/FRG-149
-  // for why this method exists.
+  @Deprecated // to be removed before 2.0
   protected void unparseListClause(SqlWriter writer, SqlNode clause) {
-    unparseListClause(writer, clause, null);
+    final SqlNodeList nodeList =
+        clause instanceof SqlNodeList
+            ? (SqlNodeList) clause
+            : SqlNodeList.of(clause);
+    writer.list(SqlWriter.FrameTypeEnum.SIMPLE, SqlWriter.COMMA, nodeList);
   }
 
+  @Deprecated // to be removed before 2.0
   protected void unparseListClause(
       SqlWriter writer,
       SqlNode clause,
       SqlKind sepKind) {
-    if (clause instanceof SqlNodeList) {
-      if (sepKind != null) {
-        ((SqlNodeList) clause).andOrList(writer, sepKind);
-      } else {
-        ((SqlNodeList) clause).commaList(writer);
-      }
+    final SqlNodeList nodeList =
+        clause instanceof SqlNodeList
+            ? (SqlNodeList) clause
+            : SqlNodeList.of(clause);
+    final SqlBinaryOperator sepOp;
+    if (sepKind == null) {
+      sepOp = SqlWriter.COMMA;
     } else {
-      clause.unparse(writer, 0, 0);
+      switch (sepKind) {
+      case AND:
+        sepOp = SqlStdOperatorTable.AND;
+        break;
+      case OR:
+        sepOp = SqlStdOperatorTable.OR;
+        break;
+      default:
+        throw new AssertionError();
+      }
     }
+    writer.list(SqlWriter.FrameTypeEnum.SIMPLE, sepOp, nodeList);
   }
 
   // override Object
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlOrderBy.java b/core/src/main/java/org/apache/calcite/sql/SqlOrderBy.java
index 57c2a71..e21fe4d 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlOrderBy.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlOrderBy.java
@@ -90,10 +90,8 @@ public class SqlOrderBy extends SqlCall {
       orderBy.query.unparse(writer, getLeftPrec(), getRightPrec());
       if (orderBy.orderList != SqlNodeList.EMPTY) {
         writer.sep(getName());
-        final SqlWriter.Frame listFrame =
-            writer.startList(SqlWriter.FrameTypeEnum.ORDER_BY_LIST);
-        unparseListClause(writer, orderBy.orderList);
-        writer.endList(listFrame);
+        writer.list(SqlWriter.FrameTypeEnum.ORDER_BY_LIST, SqlWriter.COMMA,
+            orderBy.orderList);
       }
       if (orderBy.offset != null) {
         final SqlWriter.Frame frame2 =
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSelectOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlSelectOperator.java
index 8f7f602..f55072c 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlSelectOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlSelectOperator.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.sql;
 
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.type.ReturnTypes;
 import org.apache.calcite.sql.util.SqlBasicVisitor;
@@ -133,6 +134,7 @@ public class SqlSelectOperator extends SqlOperator {
     }
   }
 
+  @SuppressWarnings("deprecation")
   public void unparse(
       SqlWriter writer,
       SqlCall call,
@@ -155,14 +157,12 @@ public class SqlSelectOperator extends SqlOperator {
       keyword.unparse(writer, 0, 0);
     }
     writer.topN(select.fetch, select.offset);
-    SqlNode selectClause = select.selectList;
-    if (selectClause == null) {
-      selectClause = SqlIdentifier.star(SqlParserPos.ZERO);
-    }
-    final SqlWriter.Frame selectListFrame =
-        writer.startList(SqlWriter.FrameTypeEnum.SELECT_LIST);
-    unparseListClause(writer, selectClause);
-    writer.endList(selectListFrame);
+    final SqlNodeList selectClause =
+        select.selectList != null
+            ? select.selectList
+            : SqlNodeList.of(SqlIdentifier.star(SqlParserPos.ZERO));
+    writer.list(SqlWriter.FrameTypeEnum.SELECT_LIST, SqlWriter.COMMA,
+        selectClause);
 
     if (select.from != null) {
       // Calcite SQL requires FROM but MySQL does not.
@@ -187,15 +187,15 @@ public class SqlSelectOperator extends SqlOperator {
         SqlNode node = select.where;
 
         // decide whether to split on ORs or ANDs
-        SqlKind whereSepKind = SqlKind.AND;
+        SqlBinaryOperator whereSep = SqlStdOperatorTable.AND;
         if ((node instanceof SqlCall)
             && node.getKind() == SqlKind.OR) {
-          whereSepKind = SqlKind.OR;
+          whereSep = SqlStdOperatorTable.OR;
         }
 
         // unroll whereClause
         final List<SqlNode> list = new ArrayList<>(0);
-        while (node.getKind() == whereSepKind) {
+        while (node.getKind() == whereSep.kind) {
           assert node instanceof SqlCall;
           final SqlCall call1 = (SqlCall) node;
           list.add(0, call1.operand(1));
@@ -203,32 +203,20 @@ public class SqlSelectOperator extends SqlOperator {
         }
         list.add(0, node);
 
-        // unparse in a WhereList frame
-        final SqlWriter.Frame whereFrame =
-            writer.startList(SqlWriter.FrameTypeEnum.WHERE_LIST);
-        unparseListClause(
-            writer,
-            new SqlNodeList(
-                list,
-                select.where.getParserPosition()),
-            whereSepKind);
-        writer.endList(whereFrame);
+        // unparse in a WHERE_LIST frame
+        writer.list(SqlWriter.FrameTypeEnum.WHERE_LIST, whereSep,
+            new SqlNodeList(list, select.where.getParserPosition()));
       } else {
         select.where.unparse(writer, 0, 0);
       }
     }
     if (select.groupBy != null) {
       writer.sep("GROUP BY");
-      final SqlWriter.Frame groupFrame =
-          writer.startList(SqlWriter.FrameTypeEnum.GROUP_BY_LIST);
-      if (select.groupBy.getList().isEmpty()) {
-        final SqlWriter.Frame frame =
-            writer.startList(SqlWriter.FrameTypeEnum.SIMPLE, "(", ")");
-        writer.endList(frame);
-      } else {
-        unparseListClause(writer, select.groupBy);
-      }
-      writer.endList(groupFrame);
+      final SqlNodeList groupBy =
+          select.groupBy.size() == 0 ? SqlNodeList.SINGLETON_EMPTY
+              : select.groupBy;
+      writer.list(SqlWriter.FrameTypeEnum.GROUP_BY_LIST, SqlWriter.COMMA,
+          groupBy);
     }
     if (select.having != null) {
       writer.sep("HAVING");
@@ -236,20 +224,13 @@ public class SqlSelectOperator extends SqlOperator {
     }
     if (select.windowDecls.size() > 0) {
       writer.sep("WINDOW");
-      final SqlWriter.Frame windowFrame =
-          writer.startList(SqlWriter.FrameTypeEnum.WINDOW_DECL_LIST);
-      for (SqlNode windowDecl : select.windowDecls) {
-        writer.sep(",");
-        windowDecl.unparse(writer, 0, 0);
-      }
-      writer.endList(windowFrame);
+      writer.list(SqlWriter.FrameTypeEnum.WINDOW_DECL_LIST, SqlWriter.COMMA,
+          select.windowDecls);
     }
     if (select.orderBy != null && select.orderBy.size() > 0) {
       writer.sep("ORDER BY");
-      final SqlWriter.Frame orderFrame =
-          writer.startList(SqlWriter.FrameTypeEnum.ORDER_BY_LIST);
-      unparseListClause(writer, select.orderBy);
-      writer.endList(orderFrame);
+      writer.list(SqlWriter.FrameTypeEnum.ORDER_BY_LIST, SqlWriter.COMMA,
+          select.orderBy);
     }
     writer.fetchOffset(select.fetch, select.offset);
     writer.endList(selectFrame);
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
index 5faaf64..b20fee9 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
@@ -48,7 +48,7 @@ public class SqlWithinGroupOperator extends SqlBinaryOperator {
     final SqlWriter.Frame orderFrame =
         writer.startList(SqlWriter.FrameTypeEnum.ORDER_BY_LIST, "(", ")");
     writer.keyword("ORDER BY");
-    ((SqlNodeList) call.operand(1)).commaList(writer);
+    ((SqlNodeList) call.operand(1)).unparse(writer, 0, 0);
     writer.endList(orderFrame);
   }
 
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWriter.java b/core/src/main/java/org/apache/calcite/sql/SqlWriter.java
index 6ecc15c..b26489e 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlWriter.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlWriter.java
@@ -16,8 +16,11 @@
  */
 package org.apache.calcite.sql;
 
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.util.SqlString;
 
+import java.util.function.Consumer;
+
 /**
  * A <code>SqlWriter</code> is the target to construct a SQL statement from a
  * parse tree. It deals with dialect differences; for example, Oracle quotes
@@ -72,6 +75,12 @@ public interface SqlWriter {
     SIMPLE,
 
     /**
+     * Comma-separated list surrounded by parentheses.
+     * The parentheses are present even if the list is empty.
+     */
+    PARENTHESES,
+
+    /**
      * The SELECT clause of a SELECT statement.
      */
     SELECT_LIST,
@@ -176,7 +185,7 @@ public interface SqlWriter {
      * <li><code>GROUP BY x, FLOOR(y)</code></li>
      * </ul>
      */
-    SUB_QUERY,
+    SUB_QUERY(true),
 
     /**
      * Set operation.
@@ -221,7 +230,22 @@ public interface SqlWriter {
      * <li><code>"A"."B"."C"</code></li>
      * </ul>
      */
-    IDENTIFIER(false);
+    IDENTIFIER(false),
+
+    /**
+     * Alias ("AS"). No indent.
+     */
+    AS(false),
+
+    /**
+     * CASE expression.
+     */
+    CASE,
+
+    /**
+     * Same behavior as user-defined frame type.
+     */
+    OTHER;
 
     private final boolean needsIndent;
 
@@ -266,6 +290,18 @@ public interface SqlWriter {
     }
   }
 
+  /** Comma operator.
+   *
+   * <p>Defined in {@code SqlWriter} because it is only used while converting
+   * {@link SqlNode} to SQL;
+   * see {@link SqlWriter#list(FrameTypeEnum, SqlBinaryOperator, SqlNodeList)}.
+   *
+   * <p>The precedence of the comma operator is low but not zero. For
+   * instance, this ensures parentheses in
+   * {@code select x, (select * from foo order by z), y from t}. */
+  SqlBinaryOperator COMMA =
+      new SqlBinaryOperator(",", SqlKind.OTHER, 2, false, null, null, null);
+
   //~ Methods ----------------------------------------------------------------
 
   /**
@@ -407,6 +443,7 @@ public interface SqlWriter {
    * Starts a list with no opening string.
    *
    * @param frameType Type of list. For example, a SELECT list will be
+   * governed according to SELECT-list formatting preferences.
    */
   Frame startList(FrameTypeEnum frameType);
 
@@ -429,6 +466,20 @@ public interface SqlWriter {
   void endList(Frame frame);
 
   /**
+   * Writes a list.
+   */
+  SqlWriter list(FrameTypeEnum frameType, Consumer<SqlWriter> action);
+
+  /**
+   * Writes a list separated by a binary operator
+   * ({@link SqlStdOperatorTable#AND AND},
+   * {@link SqlStdOperatorTable#OR OR}, or
+   * {@link #COMMA COMMA}).
+   */
+  SqlWriter list(FrameTypeEnum frameType, SqlBinaryOperator sepOp,
+      SqlNodeList list);
+
+  /**
    * Writes a list separator, unless the separator is "," and this is the
    * first occurrence in the list.
    *
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWriterConfig.java b/core/src/main/java/org/apache/calcite/sql/SqlWriterConfig.java
new file mode 100644
index 0000000..7e9c1c6
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/sql/SqlWriterConfig.java
@@ -0,0 +1,434 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you 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
+ *
+ * http://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.
+ */
+package org.apache.calcite.sql;
+
+import org.apache.calcite.sql.pretty.SqlPrettyWriter;
+import org.apache.calcite.util.ImmutableBeans;
+
+/** Configuration for {@link SqlWriter} and {@link SqlPrettyWriter}. */
+public interface SqlWriterConfig {
+  /** Returns the dialect. */
+  @ImmutableBeans.Property
+  SqlDialect dialect();
+
+  /** Sets {@link #dialect()}. */
+  SqlWriterConfig withDialect(SqlDialect dialect);
+
+  /** Returns whether to print keywords (SELECT, AS, etc.) in lower-case.
+   * Default is false: keywords are printed in upper-case. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean keywordsLowerCase();
+
+  /** Sets {@link #keywordsLowerCase}. */
+  SqlWriterConfig withKeywordsLowerCase(boolean keywordsLowerCase);
+
+  /** Returns whether to quote all identifiers, even those which would be
+   * correct according to the rules of the {@link SqlDialect} if quotation
+   * marks were omitted. Default is true. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean quoteAllIdentifiers();
+
+  /** Sets {@link #quoteAllIdentifiers}. */
+  SqlWriterConfig withQuoteAllIdentifiers(boolean quoteAllIdentifiers);
+
+  /** Returns the number of spaces indentation. Default is 4. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.IntDefault(4)
+  int indentation();
+
+  /** Sets {@link #indentation}. */
+  SqlWriterConfig withIndentation(int indentation);
+
+  /** Returns whether a clause (FROM, WHERE, GROUP BY, HAVING, WINDOW,
+   * ORDER BY) starts a new line. Default is true. SELECT is always at the
+   * start of a line. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean clauseStartsLine();
+
+  /** Sets {@link #clauseStartsLine}. */
+  SqlWriterConfig withClauseStartsLine(boolean clauseStartsLine);
+
+  /** Returns whether a clause (FROM, WHERE, GROUP BY, HAVING, WINDOW,
+   * ORDER BY) is followed by a new line. Default is false. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean clauseEndsLine();
+
+  /** Sets {@link #clauseEndsLine()}. */
+  SqlWriterConfig withClauseEndsLine(boolean clauseEndsLine);
+
+  /** Returns whether each item in a SELECT list, GROUP BY list, or ORDER BY
+   * list is on its own line.
+   *
+   * <p>Default is false;
+   * this property is superseded by {@link #selectFolding()},
+   * {@link #groupByFolding()}, {@link #orderByFolding()}. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean selectListItemsOnSeparateLines();
+
+  /** Sets {@link #selectListItemsOnSeparateLines}. */
+  SqlWriterConfig withSelectListItemsOnSeparateLines(
+      boolean selectListItemsOnSeparateLines);
+
+  /** Returns the line-folding policy for lists in the SELECT, GROUP BY and
+   * ORDER clauses, for items in the SET clause of UPDATE, and for items in
+   * VALUES.
+   *
+   * @see #foldLength()
+   *
+   * <p>If not set, the values of
+   * {@link #selectListItemsOnSeparateLines()},
+   * {@link #valuesListNewline()},
+   * {@link #updateSetListNewline()},
+   * {@link #windowDeclListNewline()} are used. */
+  @ImmutableBeans.Property
+  LineFolding lineFolding();
+
+  /** Sets {@link #lineFolding()}. */
+  SqlWriterConfig withLineFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the SELECT clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding selectFolding();
+
+  /** Sets {@link #selectFolding()}. */
+  SqlWriterConfig withSelectFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the FROM clause (and JOIN).
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.EnumDefault("TALL")
+  LineFolding fromFolding();
+
+  /** Sets {@link #fromFolding()}. */
+  SqlWriterConfig withFromFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the WHERE clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding whereFolding();
+
+  /** Sets {@link #whereFolding()}. */
+  SqlWriterConfig withWhereFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the GROUP BY clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding groupByFolding();
+
+  /** Sets {@link #groupByFolding()}. */
+  SqlWriterConfig withGroupByFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the HAVING clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding havingFolding();
+
+  /** Sets {@link #havingFolding()}. */
+  SqlWriterConfig withHavingFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the WINDOW clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding windowFolding();
+
+  /** Sets {@link #windowFolding()}. */
+  SqlWriterConfig withWindowFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the MATCH_RECOGNIZE clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding matchFolding();
+
+  /** Sets {@link #matchFolding()}. */
+  SqlWriterConfig withMatchFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the ORDER BY clause.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding orderByFolding();
+
+  /** Sets {@link #orderByFolding()}. */
+  SqlWriterConfig withOrderByFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the OVER clause or a window
+   * declaration. If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding overFolding();
+
+  /** Sets {@link #overFolding()}. */
+  SqlWriterConfig withOverFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the VALUES expression.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding valuesFolding();
+
+  /** Sets {@link #valuesFolding()}. */
+  SqlWriterConfig withValuesFolding(LineFolding lineFolding);
+
+  /** Returns the line-folding policy for the SET clause of an UPDATE statement.
+   * If not set, the value of {@link #lineFolding()} is used. */
+  @ImmutableBeans.Property
+  LineFolding updateSetFolding();
+
+  /** Sets {@link #updateSetFolding()}. */
+  SqlWriterConfig withUpdateSetFolding(LineFolding lineFolding);
+
+  /**
+   * Returns whether to use a fix for SELECT list indentations.
+   *
+   * <ul>
+   * <li>If set to "false":
+   *
+   * <blockquote><pre>
+   * SELECT
+   *     A as A,
+   *         B as B,
+   *         C as C,
+   *     D
+   * </pre></blockquote>
+   *
+   * <li>If set to "true" (the default):
+   *
+   * <blockquote><pre>
+   * SELECT
+   *     A as A,
+   *     B as B,
+   *     C as C,
+   *     D
+   * </pre></blockquote>
+   * </ul>
+   */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean selectListExtraIndentFlag();
+
+  /** Sets {@link #selectListExtraIndentFlag}. */
+  SqlWriterConfig withSelectListExtraIndentFlag(boolean selectListExtraIndentFlag);
+
+  /** Returns whether each declaration in a WINDOW clause should be on its own
+   * line.
+   *
+   * <p>Default is true;
+   * this property is superseded by {@link #windowFolding()}. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean windowDeclListNewline();
+
+  /** Sets {@link #windowDeclListNewline}. */
+  SqlWriterConfig withWindowDeclListNewline(boolean windowDeclListNewline);
+
+  /** Returns whether each row in a VALUES clause should be on its own
+   * line.
+   *
+   * <p>Default is true;
+   * this property is superseded by {@link #valuesFolding()}. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean valuesListNewline();
+
+  /** Sets {@link #valuesListNewline}. */
+  SqlWriterConfig withValuesListNewline(boolean valuesListNewline);
+
+  /** Returns whether each assignment in the SET clause of an UPDATE or MERGE
+   * statement should be on its own line.
+   *
+   * <p>Default is true;
+   * this property is superseded by {@link #updateSetFolding()}. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(true)
+  boolean updateSetListNewline();
+
+  /** Sets {@link #updateSetListNewline}. */
+  SqlWriterConfig withUpdateSetListNewline(boolean updateSetListNewline);
+
+  /** Returns whether a WINDOW clause should start its own line. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean windowNewline();
+
+  /** Sets {@link #windowNewline}. */
+  SqlWriterConfig withWindowNewline(boolean windowNewline);
+
+  /** Returns whether commas in SELECT, GROUP BY and ORDER clauses should
+   * appear at the start of the line. Default is false. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean leadingComma();
+
+  /** Sets {@link #leadingComma()}. */
+  SqlWriterConfig withLeadingComma(boolean leadingComma);
+
+  /** Returns the sub-query style.
+   * Default is {@link SqlWriter.SubQueryStyle#HYDE}. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.EnumDefault("HYDE")
+  SqlWriter.SubQueryStyle subQueryStyle();
+
+  /** Sets {@link #subQueryStyle}. */
+  SqlWriterConfig withSubQueryStyle(SqlWriter.SubQueryStyle subQueryStyle);
+
+  /** Returns whether to print a newline before each AND or OR (whichever is
+   * higher level) in WHERE clauses.
+   *
+   * <p>NOTE: Ignored when alwaysUseParentheses is set to true. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean whereListItemsOnSeparateLines();
+
+  /** Sets {@link #whereListItemsOnSeparateLines}. */
+  SqlWriterConfig withWhereListItemsOnSeparateLines(
+      boolean whereListItemsOnSeparateLines);
+
+  /** Returns whether expressions should always be included in parentheses.
+   * Default is false. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean alwaysUseParentheses();
+
+  /** Sets {@link #alwaysUseParentheses}. */
+  SqlWriterConfig withAlwaysUseParentheses(boolean alwaysUseParentheses);
+
+  /** Returns the maximum line length. Default is zero, which means there is
+   * no maximum. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.IntDefault(0)
+  int lineLength();
+
+  /** Sets {@link #lineLength}. */
+  SqlWriterConfig withLineLength(int lineLength);
+
+  /** Returns the line length at which items are chopped or folded (for clauses
+   * that have chosen {@link LineFolding#CHOP} or {@link LineFolding#FOLD}).
+   * Default is 80. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.IntDefault(80)
+  int foldLength();
+
+  /** Sets {@link #foldLength()}. */
+  SqlWriterConfig withFoldLength(int lineLength);
+
+  /** Returns whether the WHEN, THEN and ELSE clauses of a CASE expression
+   * appear at the start of a new line. The default is false. */
+  @ImmutableBeans.Property
+  @ImmutableBeans.BooleanDefault(false)
+  boolean caseClausesOnNewLines();
+
+  /** Sets {@link #caseClausesOnNewLines}. */
+  SqlWriterConfig withCaseClausesOnNewLines(boolean caseClausesOnNewLines);
+
+  /** Policy for how to do deal with long lines.
+   *
+   * <p>The following examples all have
+   * {@link #clauseEndsLine ClauseEndsLine=true},
+   * {@link #indentation Indentation=4}, and
+   * {@link #foldLength FoldLength=25} (so that the long {@code SELECT}
+   * clause folds but the shorter {@code GROUP BY} clause does not).
+   *
+   * <p>Note that {@link #clauseEndsLine ClauseEndsLine} is observed in
+   * STEP and TALL modes, and in CHOP mode when a line is long.
+   *
+   * <table border=1>
+   * <caption>SQL formatted with each Folding value</caption>
+   * <tr>
+   *   <th>Folding</th>
+   *   <th>Example</th>
+   * </tr>
+   *
+   * <tr>
+   *   <td>WIDE</td>
+   *   <td><pre>
+   * SELECT abc, def, ghi, jkl, mno, pqr
+   * FROM t
+   * GROUP BY abc, def</pre></td>
+   * </tr>
+   *
+   * <tr>
+   *   <td>STEP</td>
+   *   <td><pre>
+   * SELECT
+   *     abc, def, ghi, jkl, mno, pqr
+   * FROM t
+   * GROUP BY
+   *     abc, def</pre></td>
+   * </tr>
+   *
+   * <tr>
+   *   <td>FOLD</td>
+   *   <td><pre>
+   * SELECT abc, def, ghi,
+   *     jkl, mno, pqr
+   * FROM t
+   * GROUP BY abc, def</pre></td>
+   * </tr>
+   *
+   * <tr>
+   *   <td>CHOP</td>
+   *   <td><pre>
+   * SELECT
+   *     abc,
+   *     def,
+   *     ghi,
+   *     jkl,
+   *     mno,
+   *     pqr
+   * FROM t
+   * GROUP BY abc, def</pre></td>
+   * </tr>
+   *
+   * <tr>
+   *   <td>TALL</td>
+   *   <td><pre>
+   * SELECT
+   *     abc,
+   *     def,
+   *     ghi,
+   *     jkl,
+   *     mno,
+   *     pqr
+   * FROM t
+   * GROUP BY
+   *     abc,
+   *     def</pre></td>
+   * </tr>
+   * </table>
+   */
+  enum LineFolding {
+    /** Do not wrap. Items are on the same line, regardless of length. */
+    WIDE,
+
+    /** As {@link #WIDE} but start a new line if {@link #clauseEndsLine()}. */
+    STEP,
+
+    /** Wrap if long. Items are on the same line, but if the line's length
+     * exceeds {@link #foldLength()}, move items to the next line. */
+    FOLD,
+
+    /** Chop down if long. Items are on the same line, but if the line grows
+     * longer than {@link #foldLength()}, put all items on separate lines. */
+    CHOP,
+
+    /** Wrap always. Items are on separate lines. */
+    TALL
+  }
+}
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java
index bf87050..4a05992 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java
@@ -127,9 +127,6 @@ import static org.apache.calcite.util.Static.RESOURCE;
 public class SqlCaseOperator extends SqlOperator {
   public static final SqlCaseOperator INSTANCE = new SqlCaseOperator();
 
-  private static final SqlWriter.FrameType FRAME_TYPE =
-      SqlWriter.FrameTypeEnum.create("CASE");
-
   //~ Constructors -----------------------------------------------------------
 
   private SqlCaseOperator() {
@@ -331,7 +328,7 @@ public class SqlCaseOperator extends SqlOperator {
       int rightPrec) {
     SqlCase kase = (SqlCase) call_;
     final SqlWriter.Frame frame =
-        writer.startList(FRAME_TYPE, "CASE", "END");
+        writer.startList(SqlWriter.FrameTypeEnum.CASE, "CASE", "END");
     assert kase.whenList.size() == kase.thenList.size();
     if (kase.value != null) {
       kase.value.unparse(writer, 0, 0);
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index 8899778..755fc4c 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -74,6 +74,7 @@ import java.util.List;
  * the standard operators and functions.
  */
 public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
+
   //~ Static fields/initializers ---------------------------------------------
 
   /**
diff --git a/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java b/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java
index f5a52f8..e29d2ef 100644
--- a/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java
+++ b/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java
@@ -17,12 +17,16 @@
 package org.apache.calcite.sql.pretty;
 
 import org.apache.calcite.avatica.util.Spaces;
+import org.apache.calcite.sql.SqlBinaryOperator;
 import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.dialect.AnsiSqlDialect;
+import org.apache.calcite.sql.dialect.CalciteSqlDialect;
 import org.apache.calcite.sql.util.SqlString;
-import org.apache.calcite.util.Unsafe;
+import org.apache.calcite.util.ImmutableBeans;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.trace.CalciteLogger;
 
@@ -32,7 +36,6 @@ import com.google.common.collect.ImmutableList;
 import org.slf4j.LoggerFactory;
 
 import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.ArrayDeque;
@@ -44,6 +47,8 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
+import java.util.function.Consumer;
+import javax.annotation.Nonnull;
 
 /**
  * Pretty printer for SQL statements.
@@ -57,69 +62,189 @@ import java.util.Set;
  * <th>Description</th>
  * <th>Default</th>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setSelectListItemsOnSeparateLines SelectListItemsOnSeparateLines}
- * </td>
- * <td>Whether each item in the select clause is on its own line</td>
+ * <td>{@link SqlWriterConfig#clauseStartsLine()} ClauseStartsLine}</td>
+ * <td>Whether a clause ({@code FROM}, {@code WHERE}, {@code GROUP BY},
+ * {@code HAVING}, {@code WINDOW}, {@code ORDER BY}) starts a new line.
+ * {@code SELECT} is always at the start of a line.</td>
+ * <td>true</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#clauseEndsLine ClauseEndsLine}</td>
+ * <td>Whether a clause ({@code SELECT}, {@code FROM}, {@code WHERE},
+ * {@code GROUP BY}, {@code HAVING}, {@code WINDOW}, {@code ORDER BY}) is
+ * followed by a new line.</td>
  * <td>false</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setCaseClausesOnNewLines CaseClausesOnNewLines}</td>
+ * <td>{@link SqlWriterConfig#caseClausesOnNewLines CaseClausesOnNewLines}</td>
  * <td>Whether the WHEN, THEN and ELSE clauses of a CASE expression appear at
  * the start of a new line.</td>
  * <td>false</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setIndentation Indentation}</td>
+ * <td>{@link SqlWriterConfig#indentation Indentation}</td>
  * <td>Number of spaces to indent</td>
  * <td>4</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setKeywordsLowerCase KeywordsLowerCase}</td>
+ * <td>{@link SqlWriterConfig#keywordsLowerCase KeywordsLowerCase}</td>
  * <td>Whether to print keywords (SELECT, AS, etc.) in lower-case.</td>
  * <td>false</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #isAlwaysUseParentheses ParenthesizeAllExprs}</td>
- * <td>Whether to enclose all expressions in parentheses, even if the operator
- * has high enough precedence that the parentheses are not required.
+ * <td>{@link SqlWriterConfig#alwaysUseParentheses AlwaysUseParentheses}</td>
+ * <td><p>Whether to enclose all expressions in parentheses, even if the
+ * operator has high enough precedence that the parentheses are not required.
  *
- * <p>For example, the parentheses are required in the expression <code>(a + b)
- * c</code> because the '*' operator has higher precedence than the '+'
- * operator, and so without the parentheses, the expression would be equivalent
- * to <code>a + (b * c)</code>. The fully-parenthesized expression, <code>((a +
- * b) * c)</code> is unambiguous even if you don't know the precedence of every
- * operator.</td>
- * <td></td>
+ * <p>For example, the parentheses are required in the expression
+ * {@code (a + b) * c} because the '*' operator has higher precedence than the
+ * '+' operator, and so without the parentheses, the expression would be
+ * equivalent to {@code a + (b * c)}. The fully-parenthesized expression,
+ * {@code ((a + b) * c)} is unambiguous even if you don't know the precedence
+ * of every operator.</td>
+ * <td>false</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setQuoteAllIdentifiers QuoteAllIdentifiers}</td>
+ * <td>{@link SqlWriterConfig#quoteAllIdentifiers QuoteAllIdentifiers}</td>
  * <td>Whether to quote all identifiers, even those which would be correct
  * according to the rules of the {@link SqlDialect} if quotation marks were
  * omitted.</td>
  * <td>true</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setSelectListItemsOnSeparateLines SelectListItemsOnSeparateLines}
- * </td>
- * <td>Whether each item in the select clause is on its own line.</td>
- * <td>false</td>
- * </tr>
- * <tr>
- * <td>{@link #setSubQueryStyle SubQueryStyle}</td>
+ * <td>{@link SqlWriterConfig#subQueryStyle SubQueryStyle}</td>
  * <td>Style for formatting sub-queries. Values are:
  * {@link org.apache.calcite.sql.SqlWriter.SubQueryStyle#HYDE Hyde},
  * {@link org.apache.calcite.sql.SqlWriter.SubQueryStyle#BLACK Black}.</td>
  *
  * <td>{@link org.apache.calcite.sql.SqlWriter.SubQueryStyle#HYDE Hyde}</td>
  * </tr>
+ *
  * <tr>
- * <td>{@link #setLineLength LineLength}</td>
- * <td>Set the desired maximum length for lines (to look nice in editors,
+ * <td>{@link SqlWriterConfig#lineLength LineLength}</td>
+ * <td>The desired maximum length for lines (to look nice in editors,
  * printouts, etc.).</td>
- * <td>0</td>
+ * <td>-1 (no maximum)</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#foldLength FoldLength}</td>
+ * <td>The line length at which lines are folded or chopped down
+ * (see {@code LineFolding}). Only has an effect if clauses are marked
+ * {@link SqlWriterConfig.LineFolding#CHOP CHOP} or
+ * {@link SqlWriterConfig.LineFolding#FOLD FOLD}.</td>
+ * <td>80</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#lineFolding LineFolding}</td>
+ * <td>How long lines are to be handled. Options are lines are
+ * WIDE (do not wrap),
+ * FOLD (wrap if long),
+ * CHOP (chop down if long),
+ * and TALL (wrap always).</td>
+ * <td>WIDE</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#selectFolding() SelectFolding}</td>
+ * <td>How the {@code SELECT} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
  * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#fromFolding FromFolding}</td>
+ * <td>How the {@code FROM} clause and nested {@code JOIN} clauses are to be
+ * folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#whereFolding WhereFolding}</td>
+ * <td>How the {@code WHERE} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#groupByFolding GroupByFolding}</td>
+ * <td>How the {@code GROUP BY} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#havingFolding HavingFolding}</td>
+ * <td>How the {@code HAVING} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#orderByFolding OrderByFolding}</td>
+ * <td>How the {@code ORDER BY} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#windowFolding WindowFolding}</td>
+ * <td>How the {@code WINDOW} clause is to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#overFolding OverFolding}</td>
+ * <td>How window declarations in the {@code WINDOW} clause
+ * and in the {@code OVER} clause of aggregate functions are to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#valuesFolding ValuesFolding}</td>
+ * <td>How lists of values in the {@code VALUES} clause are to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
+ * <tr>
+ * <td>{@link SqlWriterConfig#updateSetFolding UpdateSetFolding}</td>
+ * <td>How assignments in the {@code SET} clause of an {@code UPDATE} statement
+ * are to be folded.</td>
+ * <td>{@code LineFolding}</td>
+ * </tr>
+ *
  * </table>
+ *
+ * <p>The following options exist for backwards compatibility. They are
+ * used if {@link SqlWriterConfig#lineFolding LineFolding} and clause-specific
+ * options such as {@link SqlWriterConfig#selectFolding SelectFolding} are not
+ * specified:
+ *
+ * <ul>
+ *
+ * <li>{@link SqlWriterConfig#selectListItemsOnSeparateLines SelectListItemsOnSeparateLines}
+ * replaced by {@link SqlWriterConfig#selectFolding SelectFolding},
+ * {@link SqlWriterConfig#groupByFolding GroupByFolding}, and
+ * {@link SqlWriterConfig#orderByFolding OrderByFolding};
+ *
+ * <li>{@link SqlWriterConfig#updateSetListNewline UpdateSetListNewline}
+ * replaced by {@link SqlWriterConfig#updateSetFolding UpdateSetFolding};
+ *
+ * <li>{@link SqlWriterConfig#windowDeclListNewline WindowDeclListNewline}
+ * replaced by {@link SqlWriterConfig#windowFolding WindowFolding};
+ *
+ * <li>{@link SqlWriterConfig#windowNewline WindowNewline}
+ * replaced by {@link SqlWriterConfig#overFolding OverFolding};
+ *
+ * <li>{@link SqlWriterConfig#valuesListNewline ValuesListNewline}
+ * replaced by {@link SqlWriterConfig#valuesFolding ValuesFolding}.
+ *
+ * </ul>
  */
 public class SqlPrettyWriter implements SqlWriter {
   //~ Static fields/initializers ---------------------------------------------
@@ -132,96 +257,133 @@ public class SqlPrettyWriter implements SqlWriter {
    * Bean holding the default property values.
    */
   private static final Bean DEFAULT_BEAN =
-      new SqlPrettyWriter(AnsiSqlDialect.DEFAULT).getBean();
+      new SqlPrettyWriter(SqlPrettyWriter.config()
+          .withDialect(AnsiSqlDialect.DEFAULT)).getBean();
   protected static final String NL = System.getProperty("line.separator");
 
   //~ Instance fields --------------------------------------------------------
 
   private final SqlDialect dialect;
-  private final StringWriter sw = new StringWriter();
-  protected final PrintWriter pw;
+  private final StringBuilder buf;
   private final Deque<FrameImpl> listStack = new ArrayDeque<>();
   private ImmutableList.Builder<Integer> dynamicParameters;
   protected FrameImpl frame;
   private boolean needWhitespace;
   protected String nextWhitespace;
-  protected boolean alwaysUseParentheses;
-  private boolean keywordsLowerCase;
+  private SqlWriterConfig config;
   private Bean bean;
-  private boolean quoteAllIdentifiers;
-  private int indentation;
-  private boolean clauseStartsLine;
-  private boolean selectListItemsOnSeparateLines;
-  private boolean selectListExtraIndentFlag;
   private int currentIndent;
-  private boolean windowDeclListNewline;
-  private boolean updateSetListNewline;
-  private boolean windowNewline;
-  private SubQueryStyle subQueryStyle;
-  private boolean whereListItemsOnSeparateLines;
 
-  private boolean caseClausesOnNewLines;
-  private int lineLength;
-  private int charCount;
+  private int lineStart;
 
   //~ Constructors -----------------------------------------------------------
 
+  private SqlPrettyWriter(SqlWriterConfig config,
+      StringBuilder buf, boolean ignore) {
+    this.buf = Objects.requireNonNull(buf);
+    this.dialect = Objects.requireNonNull(config.dialect());
+    this.config = Objects.requireNonNull(config);
+    lineStart = 0;
+    reset();
+  }
+
+  /** Creates a writer with the given configuration
+   * and a given buffer to write to. */
+  public SqlPrettyWriter(@Nonnull SqlWriterConfig config,
+      @Nonnull StringBuilder buf) {
+    this(config, Objects.requireNonNull(buf), false);
+  }
+
+  /** Creates a writer with the given configuration and dialect,
+   * and a given print writer (or a private print writer if it is null). */
+  public SqlPrettyWriter(
+      SqlDialect dialect,
+      SqlWriterConfig config,
+      StringBuilder buf) {
+    this(config.withDialect(Objects.requireNonNull(dialect)), buf);
+  }
+
+  /** Creates a writer with the given configuration
+   * and a private print writer. */
+  @Deprecated
+  public SqlPrettyWriter(SqlDialect dialect, SqlWriterConfig config) {
+    this(config.withDialect(Objects.requireNonNull(dialect)));
+  }
+
+  @Deprecated
   public SqlPrettyWriter(
       SqlDialect dialect,
       boolean alwaysUseParentheses,
       PrintWriter pw) {
-    if (pw == null) {
-      pw = new PrintWriter(sw);
-    }
-    this.pw = pw;
-    this.dialect = dialect;
-    this.alwaysUseParentheses = alwaysUseParentheses;
-    resetSettings();
-    reset();
+    // NOTE that 'pw' is ignored; there is no place for it in the new API
+    this(config().withDialect(Objects.requireNonNull(dialect))
+        .withAlwaysUseParentheses(alwaysUseParentheses));
   }
 
+  @Deprecated
   public SqlPrettyWriter(
       SqlDialect dialect,
       boolean alwaysUseParentheses) {
-    this(dialect, alwaysUseParentheses, null);
+    this(config().withDialect(Objects.requireNonNull(dialect))
+        .withAlwaysUseParentheses(alwaysUseParentheses));
   }
 
+  /** Creates a writer with a given dialect, the default configuration
+   * and a private print writer. */
+  @Deprecated
   public SqlPrettyWriter(SqlDialect dialect) {
-    this(dialect, true);
+    this(config().withDialect(Objects.requireNonNull(dialect)));
+  }
+
+  /** Creates a writer with the given configuration,
+   * and a private builder. */
+  public SqlPrettyWriter(@Nonnull SqlWriterConfig config) {
+    this(config, new StringBuilder(), true);
+  }
+
+  /** Creates a writer with the default configuration.
+   *
+   * @see #config() */
+  public SqlPrettyWriter() {
+    this(config());
+  }
+
+  /** Creates a {@link SqlWriterConfig} with Calcite's SQL dialect. */
+  public static SqlWriterConfig config() {
+    return ImmutableBeans.create(SqlWriterConfig.class)
+        .withDialect(CalciteSqlDialect.DEFAULT);
   }
 
   //~ Methods ----------------------------------------------------------------
 
-  /**
-   * Sets whether the WHEN, THEN and ELSE clauses of a CASE expression appear
-   * at the start of a new line. The default is false.
-   */
+  @Deprecated
   public void setCaseClausesOnNewLines(boolean caseClausesOnNewLines) {
-    this.caseClausesOnNewLines = caseClausesOnNewLines;
+    this.config = config.withCaseClausesOnNewLines(caseClausesOnNewLines);
   }
 
-  /**
-   * Sets the sub-query style. Default is
-   * {@link org.apache.calcite.sql.SqlWriter.SubQueryStyle#HYDE}.
-   */
+  @Deprecated
   public void setSubQueryStyle(SubQueryStyle subQueryStyle) {
-    this.subQueryStyle = subQueryStyle;
+    this.config = config.withSubQueryStyle(subQueryStyle);
   }
 
+  @Deprecated
   public void setWindowNewline(boolean windowNewline) {
-    this.windowNewline = windowNewline;
+    this.config = config.withWindowNewline(windowNewline);
   }
 
+  @Deprecated
   public void setWindowDeclListNewline(boolean windowDeclListNewline) {
-    this.windowDeclListNewline = windowDeclListNewline;
+    this.config = config.withWindowDeclListNewline(windowDeclListNewline);
   }
 
+  @Deprecated
   public int getIndentation() {
-    return indentation;
+    return config.indentation();
   }
 
+  @Deprecated
   public boolean isAlwaysUseParentheses() {
-    return alwaysUseParentheses;
+    return config.alwaysUseParentheses();
   }
 
   public boolean inQuery() {
@@ -231,55 +393,49 @@ public class SqlPrettyWriter implements SqlWriter {
         || (frame.frameType == FrameTypeEnum.SETOP);
   }
 
+  @Deprecated
   public boolean isQuoteAllIdentifiers() {
-    return quoteAllIdentifiers;
+    return config.quoteAllIdentifiers();
   }
 
+  @Deprecated
   public boolean isClauseStartsLine() {
-    return clauseStartsLine;
+    return config.clauseStartsLine();
   }
 
+  @Deprecated
   public boolean isSelectListItemsOnSeparateLines() {
-    return selectListItemsOnSeparateLines;
+    return config.selectListItemsOnSeparateLines();
   }
 
+  @Deprecated
   public boolean isWhereListItemsOnSeparateLines() {
-    return whereListItemsOnSeparateLines;
+    return config.whereListItemsOnSeparateLines();
   }
 
+  @Deprecated
   public boolean isSelectListExtraIndentFlag() {
-    return selectListExtraIndentFlag;
+    return config.selectListExtraIndentFlag();
   }
 
+  @Deprecated
   public boolean isKeywordsLowerCase() {
-    return keywordsLowerCase;
+    return config.keywordsLowerCase();
   }
 
+  @Deprecated
   public int getLineLength() {
-    return lineLength;
+    return config.lineLength();
   }
 
   public void resetSettings() {
     reset();
-    indentation = 4;
-    clauseStartsLine = true;
-    selectListItemsOnSeparateLines = false;
-    selectListExtraIndentFlag = true;
-    keywordsLowerCase = false;
-    quoteAllIdentifiers = true;
-    windowDeclListNewline = true;
-    updateSetListNewline = true;
-    windowNewline = false;
-    subQueryStyle = SubQueryStyle.HYDE;
-    alwaysUseParentheses = false;
-    whereListItemsOnSeparateLines = false;
-    lineLength = 0;
-    charCount = 0;
+    config = config();
   }
 
   public void reset() {
-    pw.flush();
-    Unsafe.clear(sw);
+    buf.setLength(0);
+    lineStart = 0;
     dynamicParameters = null;
     setNeedWhitespace(false);
     nextWhitespace = " ";
@@ -295,13 +451,9 @@ public class SqlPrettyWriter implements SqlWriter {
     return bean;
   }
 
-  /**
-   * Sets the number of spaces indentation.
-   *
-   * @see #getIndentation()
-   */
+  @Deprecated
   public void setIndentation(int indentation) {
-    this.indentation = indentation;
+    this.config = config.withIndentation(indentation);
   }
 
   /**
@@ -343,78 +495,47 @@ public class SqlPrettyWriter implements SqlWriter {
     }
   }
 
-  /**
-   * Sets whether a clause (FROM, WHERE, GROUP BY, HAVING, WINDOW, ORDER BY)
-   * starts a new line. Default is true. SELECT is always at the start of a
-   * line.
-   */
+  @Deprecated
   public void setClauseStartsLine(boolean clauseStartsLine) {
-    this.clauseStartsLine = clauseStartsLine;
+    this.config = config.withClauseStartsLine(clauseStartsLine);
   }
 
-  /**
-   * Sets whether each item in a SELECT list, GROUP BY list, or ORDER BY list
-   * is on its own line. Default false.
-   */
+  @Deprecated
   public void setSelectListItemsOnSeparateLines(boolean b) {
-    this.selectListItemsOnSeparateLines = b;
+    this.config = config.withSelectListItemsOnSeparateLines(b);
   }
 
-  /**
-   * Sets whether to use a fix for SELECT list indentations.
-   *
-   * <ul>
-   * <li>If set to "false":
-   *
-   * <blockquote><pre>
-   * SELECT
-   *     A as A
-   *         B as B
-   *         C as C
-   *     D
-   * </pre></blockquote>
-   *
-   * <li>If set to "true":
-   *
-   * <blockquote><pre>
-   * SELECT
-   *     A as A
-   *     B as B
-   *     C as C
-   *     D
-   * </pre></blockquote>
-   * </ul>
-   */
-  public void setSelectListExtraIndentFlag(boolean b) {
-    this.selectListExtraIndentFlag = b;
+  @Deprecated
+  public void setSelectListExtraIndentFlag(boolean selectListExtraIndentFlag) {
+    this.config =
+        config.withSelectListExtraIndentFlag(selectListExtraIndentFlag);
   }
 
-  /**
-   * Sets whether to print keywords (SELECT, AS, etc.) in lower-case. The
-   * default is false: keywords are printed in upper-case.
-   */
-  public void setKeywordsLowerCase(boolean b) {
-    this.keywordsLowerCase = b;
+  @Deprecated
+  public void setKeywordsLowerCase(boolean keywordsLowerCase) {
+    this.config = config.withKeywordsLowerCase(keywordsLowerCase);
   }
 
-  /**
-   * Sets whether to print a newline before each AND or OR (whichever is
-   * higher level) in WHERE clauses. NOTE: <i>Ignored when
-   * alwaysUseParentheses is set to true.</i>
-   */
-
-  public void setWhereListItemsOnSeparateLines(boolean b) {
-    this.whereListItemsOnSeparateLines = b;
+  @Deprecated
+  public void setWhereListItemsOnSeparateLines(
+      boolean whereListItemsOnSeparateLines) {
+    this.config =
+        config.withWhereListItemsOnSeparateLines(whereListItemsOnSeparateLines);
   }
 
-  public void setAlwaysUseParentheses(boolean b) {
-    this.alwaysUseParentheses = b;
+  @Deprecated
+  public void setAlwaysUseParentheses(boolean alwaysUseParentheses) {
+    this.config = config.withAlwaysUseParentheses(alwaysUseParentheses);
   }
 
   public void newlineAndIndent() {
-    pw.println();
-    charCount = 0;
-    indent(currentIndent);
+    newlineAndIndent(currentIndent);
+  }
+
+  public void newlineAndIndent(int indent) {
+    buf.append(NL);
+    lineStart = buf.length();
+    indent(indent);
     setNeedWhitespace(false); // no further whitespace necessary
   }
 
@@ -422,19 +543,12 @@ public class SqlPrettyWriter implements SqlWriter {
     if (indent < 0) {
       throw new IllegalArgumentException("negative indent " + indent);
     }
-    Spaces.append(pw, indent);
-    charCount += indent;
+    Spaces.append(buf, indent);
   }
 
-  /**
-   * Sets whether to quote all identifiers, even those which would be correct
-   * according to the rules of the {@link SqlDialect} if quotation marks were
-   * omitted.
-   *
-   * <p>Default true.
-   */
-  public void setQuoteAllIdentifiers(boolean b) {
-    this.quoteAllIdentifiers = b;
+  @Deprecated
+  public void setQuoteAllIdentifiers(boolean quoteAllIdentifiers) {
+    this.config = config.withQuoteAllIdentifiers(quoteAllIdentifiers);
   }
 
   /**
@@ -454,283 +568,265 @@ public class SqlPrettyWriter implements SqlWriter {
       String keyword,
       String open,
       String close) {
-    int indentation = getIndentation();
-    if (frameType instanceof FrameTypeEnum) {
-      FrameTypeEnum frameTypeEnum = (FrameTypeEnum) frameType;
-
-      switch (frameTypeEnum) {
-      case WINDOW_DECL_LIST:
-      case VALUES:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            false,
-            indentation,
-            windowDeclListNewline,
-            false,
-            false);
-
-      case UPDATE_SET_LIST:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            updateSetListNewline,
-            indentation,
-            false,
-            false,
-            false);
-
-      case SELECT_LIST:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            selectListExtraIndentFlag ? indentation : 0,
-            selectListItemsOnSeparateLines,
-            false,
-            indentation,
-            selectListItemsOnSeparateLines,
-            false,
-            false);
-
-      case ORDER_BY_LIST:
-      case GROUP_BY_LIST:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            selectListItemsOnSeparateLines,
-            false,
-            indentation,
-            selectListItemsOnSeparateLines,
-            false,
-            false);
-
-      case SUB_QUERY:
-        switch (subQueryStyle) {
-        case BLACK:
-
-          // Generate, e.g.:
-          //
-          // WHERE foo = bar IN
-          // (   SELECT ...
-          open = Spaces.padRight("(", indentation);
-          return new FrameImpl(
-              frameType,
-              keyword,
-              open,
-              close,
-              0,
-              false,
-              true,
-              indentation,
-              false,
-              false,
-              false) {
-            protected void _before() {
-              newlineAndIndent();
-            }
-          };
-        case HYDE:
-
-          // Generate, e.g.:
-          //
-          // WHERE foo IN (
-          //     SELECT ...
-          return new FrameImpl(
-              frameType,
-              keyword,
-              open,
-              close,
-              0,
-              false,
-              true,
-              0,
-              false,
-              false,
-              false) {
-            protected void _before() {
-              nextWhitespace = NL;
-            }
-          };
-        default:
-          throw Util.unexpected(subQueryStyle);
-        }
-
-      case ORDER_BY:
-      case OFFSET:
-      case FETCH:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            0,
-            false,
-            true,
-            0,
-            false,
-            false,
-            false);
-
-      case SELECT:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            isClauseStartsLine(), // newline before FROM, WHERE etc.
-            0, // all clauses appear below SELECT
-            false,
-            false,
-            false);
-
-      case SETOP:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            isClauseStartsLine(), // newline before UNION, EXCEPT
-            0, // all clauses appear below SELECT
-            isClauseStartsLine(), // newline after UNION, EXCEPT
-            false,
-            false);
-
-      case WINDOW:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            windowNewline,
-            0,
-            false,
-            false,
-            false);
-
-      case FUN_CALL:
-        setNeedWhitespace(false);
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            false,
-            indentation,
-            false,
-            false,
-            false);
-
-      case IDENTIFIER:
-      case SIMPLE:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            false,
-            indentation,
-            false,
-            false,
-            false);
-
-      case WHERE_LIST:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            whereListItemsOnSeparateLines,
-            0,
-            false,
-            false,
-            false);
-
-      case FROM_LIST:
-      case JOIN:
-        return new FrameImpl(
-            frameType,
-            keyword,
-            open,
-            close,
-            indentation,
-            false,
-            isClauseStartsLine(), // newline before UNION, EXCEPT
-            0, // all clauses appear below SELECT
-            isClauseStartsLine(), // newline after UNION, EXCEPT
-            false,
-            false) {
-          protected void sep(boolean printFirst, String sep) {
-            boolean newlineBefore =
-                newlineBeforeSep
-                    && !sep.equals(",");
-            boolean newlineAfter =
-                newlineAfterSep && sep.equals(",");
-            if ((itemCount > 0) || printFirst) {
-              if (newlineBefore && (itemCount > 0)) {
-                pw.println();
-                charCount = 0;
-                indent(currentIndent + sepIndent);
-                setNeedWhitespace(false);
-              }
-              keyword(sep);
-              nextWhitespace = newlineAfter ? NL : " ";
-            }
-            ++itemCount;
-          }
-        };
-      default:
-        // fall through
-      }
-    }
+    final FrameTypeEnum frameTypeEnum =
+        frameType instanceof FrameTypeEnum ? (FrameTypeEnum) frameType
+            : FrameTypeEnum.OTHER;
+    final int indentation = config.indentation();
     boolean newlineAfterOpen = false;
     boolean newlineBeforeSep = false;
+    boolean newlineAfterSep = false;
     boolean newlineBeforeClose = false;
+    int left = column();
     int sepIndent = indentation;
-    if (frameType.getName().equals("CASE")) {
-      if (caseClausesOnNewLines) {
-        newlineAfterOpen = true;
+    int extraIndent = 0;
+    final SqlWriterConfig.LineFolding fold = fold(frameTypeEnum);
+    final boolean newline = fold == SqlWriterConfig.LineFolding.TALL;
+
+    switch (frameTypeEnum) {
+    case SELECT:
+      extraIndent = indentation;
+      newlineAfterOpen = false;
+      newlineBeforeSep = config.clauseStartsLine(); // newline before FROM, WHERE etc.
+      newlineAfterSep = false;
+      sepIndent = 0; // all clauses appear below SELECT
+      break;
+
+    case SETOP:
+      extraIndent = 0;
+      newlineAfterOpen = false;
+      newlineBeforeSep = config.clauseStartsLine(); // newline before UNION, EXCEPT
+      newlineAfterSep = config.clauseStartsLine(); // newline after UNION, EXCEPT
+      sepIndent = 0; // all clauses appear below SELECT
+      break;
+
+    case SELECT_LIST:
+    case FROM_LIST:
+    case JOIN:
+    case GROUP_BY_LIST:
+    case ORDER_BY_LIST:
+    case WINDOW_DECL_LIST:
+    case VALUES:
+      if (config.selectListExtraIndentFlag()) {
+        extraIndent = indentation;
+      }
+      left = frame == null ? 0 : frame.left;
+      newlineAfterOpen = config.clauseEndsLine()
+          && (fold == SqlWriterConfig.LineFolding.TALL
+              || fold == SqlWriterConfig.LineFolding.STEP);
+      newlineBeforeSep = false;
+      newlineAfterSep = newline;
+      if (config.leadingComma() && newline) {
         newlineBeforeSep = true;
-        newlineBeforeClose = true;
-        sepIndent = 0;
+        newlineAfterSep = false;
+        sepIndent = -", ".length();
       }
+      break;
+
+    case WHERE_LIST:
+    case WINDOW:
+      extraIndent = indentation;
+      newlineAfterOpen = newline && config.clauseEndsLine();
+      newlineBeforeSep = newline;
+      sepIndent = 0;
+      newlineAfterSep = false;
+      break;
+
+    case ORDER_BY:
+    case OFFSET:
+    case FETCH:
+      newlineAfterOpen = false;
+      newlineBeforeSep = true;
+      sepIndent = 0;
+      newlineAfterSep = false;
+      break;
+
+    case UPDATE_SET_LIST:
+      extraIndent = indentation;
+      newlineAfterOpen = newline;
+      newlineBeforeSep = false;
+      sepIndent = 0;
+      newlineAfterSep = newline;
+      break;
+
+    case CASE:
+      newlineAfterOpen = newline;
+      newlineBeforeSep = newline;
+      newlineBeforeClose = newline;
+      sepIndent = 0;
+      break;
+    }
+
+    final int chopColumn;
+    final SqlWriterConfig.LineFolding lineFolding;
+    if (config.lineFolding() == null) {
+      lineFolding = SqlWriterConfig.LineFolding.WIDE;
+      chopColumn = -1;
+    } else {
+      lineFolding = config.lineFolding();
+      if (config.foldLength() > 0
+          && (lineFolding == SqlWriterConfig.LineFolding.CHOP
+              || lineFolding == SqlWriterConfig.LineFolding.FOLD
+              || lineFolding == SqlWriterConfig.LineFolding.STEP)) {
+        chopColumn = left + config.foldLength();
+      } else {
+        chopColumn = -1;
+      }
+    }
+
+    switch (frameTypeEnum) {
+    case UPDATE_SET_LIST:
+    case WINDOW_DECL_LIST:
+    case VALUES:
+    case SELECT:
+    case SETOP:
+    case SELECT_LIST:
+    case WHERE_LIST:
+    case ORDER_BY_LIST:
+    case GROUP_BY_LIST:
+    case WINDOW:
+    case ORDER_BY:
+    case OFFSET:
+    case FETCH:
+      return new FrameImpl(frameType, keyword, open, close,
+          left, extraIndent, chopColumn, lineFolding, newlineAfterOpen,
+          newlineBeforeSep, sepIndent, newlineAfterSep, false, false);
+
+    case SUB_QUERY:
+      switch (config.subQueryStyle()) {
+      case BLACK:
+        // Generate, e.g.:
+        //
+        // WHERE foo = bar IN
+        // (   SELECT ...
+        open = Spaces.padRight("(", indentation - 1);
+        return new FrameImpl(frameType, keyword, open, close,
+            left, 0, chopColumn, lineFolding, false, true, indentation,
+            false, false, false) {
+          protected void _before() {
+            newlineAndIndent();
+          }
+        };
+
+      case HYDE:
+        // Generate, e.g.:
+        //
+        // WHERE foo IN (
+        //     SELECT ...
+        return new FrameImpl(frameType, keyword, open, close, left, 0,
+            chopColumn, lineFolding, false, true, 0, false, false, false) {
+          protected void _before() {
+            nextWhitespace = NL;
+          }
+        };
+
+      default:
+        throw Util.unexpected(config.subQueryStyle());
+      }
+
+    case FUN_CALL:
+      setNeedWhitespace(false);
+      return new FrameImpl(frameType, keyword, open, close,
+          left, indentation, chopColumn, lineFolding, false, false,
+          indentation, false, false, false);
+
+    case PARENTHESES:
+      open = "(";
+      close = ")";
+      // fall through
+    case IDENTIFIER:
+    case SIMPLE:
+      return new FrameImpl(frameType, keyword, open, close,
+          left, indentation, chopColumn, lineFolding, false, false,
+          indentation, false, false, false);
+
+    case FROM_LIST:
+    case JOIN:
+      newlineBeforeSep = newline;
+      sepIndent = 0; // all clauses appear below SELECT
+      newlineAfterSep = false;
+      final boolean newlineBeforeComma = config.leadingComma() && newline;
+      final boolean newlineAfterComma = !config.leadingComma() && newline;
+      return new FrameImpl(frameType, keyword, open, close, left,
+          indentation, chopColumn, lineFolding, newlineAfterOpen,
+          newlineBeforeSep, sepIndent, newlineAfterSep, false, false) {
+        protected void sep(boolean printFirst, String sep) {
+          final boolean newlineBeforeSep;
+          final boolean newlineAfterSep;
+          if (sep.equals(",")) {
+            newlineBeforeSep = newlineBeforeComma;
+            newlineAfterSep = newlineAfterComma;
+          } else {
+            newlineBeforeSep = this.newlineBeforeSep;
+            newlineAfterSep = this.newlineAfterSep;
+          }
+          if (itemCount == 0) {
+            if (newlineAfterOpen) {
+              newlineAndIndent(currentIndent);
+            }
+          } else {
+            if (newlineBeforeSep) {
+              newlineAndIndent(currentIndent + sepIndent);
+            }
+          }
+          if ((itemCount > 0) || printFirst) {
+            keyword(sep);
+            nextWhitespace = newlineAfterSep ? NL : " ";
+          }
+          ++itemCount;
+        }
+      };
+
+    default:
+    case OTHER:
+      return new FrameImpl(frameType, keyword, open, close, left, indentation,
+          chopColumn, lineFolding, newlineAfterOpen, newlineBeforeSep,
+          sepIndent, newlineAfterSep, newlineBeforeClose, false);
+    }
+  }
+
+  private SqlWriterConfig.LineFolding fold(FrameTypeEnum frameType) {
+    switch (frameType) {
+    case SELECT_LIST:
+      return f3(config.selectFolding(), config.lineFolding(),
+          config.selectListItemsOnSeparateLines());
+    case GROUP_BY_LIST:
+      return f3(config.groupByFolding(), config.lineFolding(),
+          config.selectListItemsOnSeparateLines());
+    case ORDER_BY_LIST:
+      return f3(config.orderByFolding(), config.lineFolding(),
+          config.selectListItemsOnSeparateLines());
+    case UPDATE_SET_LIST:
+      return f3(config.updateSetFolding(), config.lineFolding(),
+          config.updateSetListNewline());
+    case WHERE_LIST:
+      return f3(config.whereFolding(), config.lineFolding(),
+          config.whereListItemsOnSeparateLines());
+    case WINDOW_DECL_LIST:
+      return f3(config.windowFolding(), config.lineFolding(),
+          config.clauseStartsLine() && config.windowDeclListNewline());
+    case WINDOW:
+      return f3(config.overFolding(), config.lineFolding(),
+          config.windowNewline());
+    case VALUES:
+      return f3(config.valuesFolding(), config.lineFolding(),
+          config.valuesListNewline());
+    case FROM_LIST:
+    case JOIN:
+      return f3(config.fromFolding(), config.lineFolding(),
+          config.caseClausesOnNewLines());
+    case CASE:
+      return f3(null, null, config.caseClausesOnNewLines());
+    default:
+      return SqlWriterConfig.LineFolding.WIDE;
     }
-    return new FrameImpl(
-        frameType,
-        keyword,
-        open,
-        close,
-        indentation,
-        newlineAfterOpen,
-        newlineBeforeSep,
-        sepIndent,
-        false,
-        newlineBeforeClose,
-        false);
+  }
+
+  private SqlWriterConfig.LineFolding f3(SqlWriterConfig.LineFolding folding0,
+      SqlWriterConfig.LineFolding folding1, boolean opt) {
+    return folding0 != null ? folding0
+        : folding1 != null ? folding1
+            : opt ? SqlWriterConfig.LineFolding.TALL
+                : SqlWriterConfig.LineFolding.WIDE;
   }
 
   /**
@@ -749,16 +845,19 @@ public class SqlPrettyWriter implements SqlWriter {
       String close) {
     assert frameType != null;
     if (frame != null) {
-      ++frame.itemCount;
+      if (frame.itemCount++ == 0 && frame.newlineAfterOpen) {
+        newlineAndIndent();
+      } else if (frameType == FrameTypeEnum.SUB_QUERY
+          && config.subQueryStyle() == SubQueryStyle.BLACK) {
+        newlineAndIndent(currentIndent - frame.extraIndent);
+      }
 
       // REVIEW jvs 9-June-2006:  This is part of the fix for FRG-149
       // (extra frame for identifier was leading to extra indentation,
       // causing select list to come out raggedy with identifiers
       // deeper than literals); are there other frame types
       // for which extra indent should be suppressed?
-      if (frameType.needsIndent()) {
-        currentIndent += frame.extraIndent;
-      }
+      currentIndent += frame.extraIndent(frameType);
       assert !listStack.contains(frame);
       listStack.push(frame);
     }
@@ -793,9 +892,7 @@ public class SqlPrettyWriter implements SqlWriter {
       assert currentIndent == 0 : currentIndent;
     } else {
       this.frame = listStack.pop();
-      if (endedFrame.frameType.needsIndent()) {
-        currentIndent -= this.frame.extraIndent;
-      }
+      currentIndent -= this.frame.extraIndent(endedFrame.frameType);
     }
   }
 
@@ -807,8 +904,7 @@ public class SqlPrettyWriter implements SqlWriter {
   }
 
   public String toString() {
-    pw.flush();
-    return sw.toString();
+    return buf.toString();
   }
 
   public SqlString toSqlString() {
@@ -828,11 +924,10 @@ public class SqlPrettyWriter implements SqlWriter {
 
   public void keyword(String s) {
     maybeWhitespace(s);
-    pw.print(
+    buf.append(
         isKeywordsLowerCase()
             ? s.toLowerCase(Locale.ROOT)
             : s.toUpperCase(Locale.ROOT));
-    charCount += s.length();
     if (!s.equals("")) {
       setNeedWhitespace(needWhitespaceAfter(s));
     }
@@ -864,19 +959,24 @@ public class SqlPrettyWriter implements SqlWriter {
       if (nextWhitespace.equals(NL)) {
         newlineAndIndent();
       } else {
-        pw.print(nextWhitespace);
-        charCount += nextWhitespace.length();
+        buf.append(nextWhitespace);
       }
       nextWhitespace = " ";
       setNeedWhitespace(false);
     }
   }
 
+  /** Returns the number of characters appended since the last newline. */
+  private int column() {
+    return buf.length() - lineStart;
+  }
+
   protected boolean tooLong(String s) {
+    final int lineLength = config.lineLength();
     boolean result =
         lineLength > 0
-            && (charCount > currentIndent)
-            && ((charCount + s.length()) >= lineLength);
+            && (column() > currentIndent)
+            && ((column() + s.length()) >= lineLength);
     if (result) {
       nextWhitespace = NL;
     }
@@ -886,14 +986,12 @@ public class SqlPrettyWriter implements SqlWriter {
 
   public void print(String s) {
     maybeWhitespace(s);
-    pw.print(s);
-    charCount += s.length();
+    buf.append(s);
   }
 
   public void print(int x) {
     maybeWhitespace("0");
-    pw.print(x);
-    charCount += String.valueOf(x).length();
+    buf.append(x);
   }
 
   public void identifier(String name, boolean quoted) {
@@ -904,8 +1002,7 @@ public class SqlPrettyWriter implements SqlWriter {
       qName = dialect.quoteIdentifier(name);
     }
     maybeWhitespace(qName);
-    pw.print(qName);
-    charCount += qName.length();
+    buf.append(qName);
     setNeedWhitespace(true);
   }
 
@@ -956,6 +1053,23 @@ public class SqlPrettyWriter implements SqlWriter {
     return startList(frameType, null, open, close);
   }
 
+  public SqlWriter list(FrameTypeEnum frameType, Consumer<SqlWriter> action) {
+    final SqlWriter.Frame selectListFrame =
+        startList(SqlWriter.FrameTypeEnum.SELECT_LIST);
+    final SqlWriter w = this;
+    action.accept(w);
+    endList(selectListFrame);
+    return this;
+  }
+
+  public SqlWriter list(FrameTypeEnum frameType, SqlBinaryOperator sepOp,
+      SqlNodeList list) {
+    final SqlWriter.Frame frame = startList(frameType);
+    ((FrameImpl) frame).list(list, sepOp);
+    endList(frame);
+    return this;
+  }
+
   public void sep(String sep) {
     sep(sep, !(sep.equals(",") || sep.equals(".")));
   }
@@ -974,8 +1088,9 @@ public class SqlPrettyWriter implements SqlWriter {
     this.needWhitespace = needWhitespace;
   }
 
+  @Deprecated
   public void setLineLength(int lineLength) {
-    this.lineLength = lineLength;
+    this.config = config.withLineLength(lineLength);
   }
 
   public void setFormatOptions(SqlFormatOptions options) {
@@ -1008,6 +1123,7 @@ public class SqlPrettyWriter implements SqlWriter {
     final String open;
     final String close;
 
+    private final int left;
     /**
      * Indent of sub-frame with respect to this one.
      */
@@ -1033,33 +1149,40 @@ public class SqlPrettyWriter implements SqlWriter {
      * Whether to print a newline after each separator.
      */
     public final boolean newlineAfterSep;
-    private final boolean newlineBeforeClose;
-    private final boolean newlineAfterClose;
-    private final boolean newlineAfterOpen;
-
-    FrameImpl(
-        FrameType frameType,
-        String keyword,
-        String open,
-        String close,
-        int extraIndent,
-        boolean newlineAfterOpen,
-        boolean newlineBeforeSep,
-        int sepIndent,
-        boolean newlineAfterSep,
-        boolean newlineBeforeClose,
-        boolean newlineAfterClose) {
+    protected final boolean newlineBeforeClose;
+    protected final boolean newlineAfterClose;
+    protected final boolean newlineAfterOpen;
+
+    /** Character count after which we should move an item to the
+     * next line. Or {@link Integer#MAX_VALUE} if we are not chopping. */
+    private final int chopLimit;
+
+    /** How lines are to be folded. */
+    private final SqlWriterConfig.LineFolding lineFolding;
+
+    FrameImpl(FrameType frameType, String keyword, String open, String close,
+        int left, int extraIndent, int chopLimit,
+        SqlWriterConfig.LineFolding lineFolding, boolean newlineAfterOpen,
+        boolean newlineBeforeSep, int sepIndent, boolean newlineAfterSep,
+        boolean newlineBeforeClose, boolean newlineAfterClose) {
       this.frameType = frameType;
       this.keyword = keyword;
       this.open = open;
       this.close = close;
+      this.left = left;
       this.extraIndent = extraIndent;
+      this.chopLimit = chopLimit;
+      this.lineFolding = lineFolding;
       this.newlineAfterOpen = newlineAfterOpen;
       this.newlineBeforeSep = newlineBeforeSep;
       this.newlineAfterSep = newlineAfterSep;
       this.newlineBeforeClose = newlineBeforeClose;
       this.newlineAfterClose = newlineAfterClose;
       this.sepIndent = sepIndent;
+      assert chopLimit >= 0
+          == (lineFolding == SqlWriterConfig.LineFolding.CHOP
+          || lineFolding == SqlWriterConfig.LineFolding.FOLD
+          || lineFolding == SqlWriterConfig.LineFolding.STEP);
     }
 
     protected void before() {
@@ -1072,9 +1195,14 @@ public class SqlPrettyWriter implements SqlWriter {
     }
 
     protected void sep(boolean printFirst, String sep) {
-      if ((newlineBeforeSep && (itemCount > 0))
-          || (newlineAfterOpen && (itemCount == 0))) {
-        newlineAndIndent();
+      if (itemCount == 0) {
+        if (newlineAfterOpen) {
+          newlineAndIndent(currentIndent);
+        }
+      } else {
+        if (newlineBeforeSep) {
+          newlineAndIndent(currentIndent + sepIndent);
+        }
       }
       if ((itemCount > 0) || printFirst) {
         keyword(sep);
@@ -1082,6 +1210,150 @@ public class SqlPrettyWriter implements SqlWriter {
       }
       ++itemCount;
     }
+
+    /** Returns the extra indent required for a given type of sub-frame. */
+    int extraIndent(FrameType subFrameType) {
+      if (this.frameType == FrameTypeEnum.ORDER_BY
+          && subFrameType == FrameTypeEnum.ORDER_BY_LIST) {
+        return config.indentation();
+      }
+      if (subFrameType.needsIndent()) {
+        return extraIndent;
+      }
+      return 0;
+    }
+
+    void list(SqlNodeList list, SqlBinaryOperator sepOp) {
+      final Save save;
+      switch (lineFolding) {
+      case CHOP:
+      case FOLD:
+        save = new Save();
+        if (!list2(list, sepOp)) {
+          save.restore();
+          final boolean newlineAfterOpen = config.clauseEndsLine();
+          final SqlWriterConfig.LineFolding lineFolding;
+          final int chopLimit;
+          if (this.lineFolding == SqlWriterConfig.LineFolding.CHOP) {
+            lineFolding = SqlWriterConfig.LineFolding.TALL;
+            chopLimit = -1;
+          } else {
+            lineFolding = SqlWriterConfig.LineFolding.FOLD;
+            chopLimit = this.chopLimit;
+          }
+          final boolean newline =
+              lineFolding == SqlWriterConfig.LineFolding.TALL;
+          final boolean newlineBeforeSep;
+          final boolean newlineAfterSep;
+          final int sepIndent;
+          if (config.leadingComma() && newline) {
+            newlineBeforeSep = true;
+            newlineAfterSep = false;
+            sepIndent = -", ".length();
+          } else if (newline) {
+            newlineBeforeSep = false;
+            newlineAfterSep = true;
+            sepIndent = this.sepIndent;
+          } else {
+            newlineBeforeSep = false;
+            newlineAfterSep = false;
+            sepIndent = this.sepIndent;
+          }
+          final FrameImpl frame2 =
+              new FrameImpl(frameType, keyword, open, close, left, extraIndent,
+                  chopLimit, lineFolding, newlineAfterOpen, newlineBeforeSep,
+                  sepIndent, newlineAfterSep, newlineBeforeClose,
+                  newlineAfterClose);
+          frame2.list2(list, sepOp);
+        }
+        break;
+      default:
+        list2(list, sepOp);
+      }
+    }
+
+    /** Tries to write a list. If the line runs too long, returns false,
+     * indicating to retry. */
+    private boolean list2(SqlNodeList list, SqlBinaryOperator sepOp) {
+      // The precedence pulling on the LHS of a node is the
+      // right-precedence of the separator operator. Similarly RHS.
+      //
+      // At the start and end of the list precedence should be 0, but non-zero
+      // precedence is useful, because it forces parentheses around
+      // sub-queries and empty lists, e.g. "SELECT a, (SELECT * FROM t), b",
+      // "GROUP BY ()".
+      final int lprec = sepOp.getRightPrec();
+      final int rprec = sepOp.getLeftPrec();
+      if (chopLimit < 0) {
+        for (int i = 0; i < list.size(); i++) {
+          SqlNode node = list.get(i);
+          sep(false, sepOp.getName());
+          node.unparse(SqlPrettyWriter.this, lprec, rprec);
+        }
+      } else if (newlineBeforeSep) {
+        for (int i = 0; i < list.size(); i++) {
+          SqlNode node = list.get(i);
+          sep(false, sepOp.getName());
+          final Save prevSize = new Save();
+          node.unparse(SqlPrettyWriter.this, lprec, rprec);
+          if (column() > chopLimit) {
+            if (lineFolding == SqlWriterConfig.LineFolding.CHOP
+                || lineFolding == SqlWriterConfig.LineFolding.TALL) {
+              return false;
+            }
+            prevSize.restore();
+            newlineAndIndent();
+            node.unparse(SqlPrettyWriter.this, lprec, rprec);
+          }
+        }
+      } else {
+        for (int i = 0; i < list.size(); i++) {
+          SqlNode node = list.get(i);
+          if (i == 0) {
+            sep(false, sepOp.getName());
+          }
+          final Save save = new Save();
+          node.unparse(SqlPrettyWriter.this, lprec, rprec);
+          if (i + 1 < list.size()) {
+            sep(false, sepOp.getName());
+          }
+          if (column() > chopLimit) {
+            switch (lineFolding) {
+            case CHOP:
+              return false;
+            case FOLD:
+              if (newlineAfterOpen != config.clauseEndsLine()) {
+                return false;
+              }
+            }
+            save.restore();
+            newlineAndIndent();
+            node.unparse(SqlPrettyWriter.this, lprec, rprec);
+            if (i + 1 < list.size()) {
+              sep(false, sepOp.getName());
+            }
+          }
+        }
+      }
+      return true;
+    }
+
+    /** Remembers the state of the current frame and writer.
+     *
+     * <p>You can call {@link #restore} to restore to that state, or just
+     * continue. It is useful if you wish to re-try with different options
+     * (for example, with lines wrapped). */
+    class Save {
+      final int bufLength;
+
+      Save() {
+        this.bufLength = buf.length();
+      }
+
+      void restore() {
+        buf.setLength(bufLength);
+      }
+    }
   }
 
   /**
@@ -1160,4 +1432,5 @@ public class SqlPrettyWriter implements SqlWriter {
       return names.toArray(new String[0]);
     }
   }
+
 }
diff --git a/core/src/main/java/org/apache/calcite/sql/type/IntervalSqlType.java b/core/src/main/java/org/apache/calcite/sql/type/IntervalSqlType.java
index f427a31..c58240c 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/IntervalSqlType.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/IntervalSqlType.java
@@ -22,6 +22,7 @@ import org.apache.calcite.rel.type.RelDataTypeFactoryImpl;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
 import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.SqlIntervalQualifier;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.dialect.AnsiSqlDialect;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
@@ -58,10 +59,12 @@ public class IntervalSqlType extends AbstractSqlType {
   protected void generateTypeString(StringBuilder sb, boolean withDetail) {
     sb.append("INTERVAL ");
     final SqlDialect dialect = AnsiSqlDialect.DEFAULT;
-    final SqlPrettyWriter writer = new SqlPrettyWriter(dialect);
-    writer.setAlwaysUseParentheses(false);
-    writer.setSelectListItemsOnSeparateLines(false);
-    writer.setIndentation(0);
+    final SqlWriterConfig config = SqlPrettyWriter.config()
+        .withAlwaysUseParentheses(false)
+        .withSelectListItemsOnSeparateLines(false)
+        .withIndentation(0)
+        .withDialect(dialect);
+    final SqlPrettyWriter writer = new SqlPrettyWriter(config);
     intervalQualifier.unparse(writer, 0, 0);
     final String sql = writer.toString();
     sb.append(new SqlString(dialect, sql).getSql());
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 9331b44..269bcb2 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -26,7 +26,8 @@ import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlSetOption;
-import org.apache.calcite.sql.dialect.CalciteSqlDialect;
+import org.apache.calcite.sql.SqlWriterConfig;
+import org.apache.calcite.sql.dialect.AnsiSqlDialect;
 import org.apache.calcite.sql.parser.impl.SqlParserImpl;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
 import org.apache.calcite.sql.test.SqlTests;
@@ -59,6 +60,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Random;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.function.Consumer;
@@ -70,6 +72,7 @@ import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assumptions.assumeFalse;
@@ -566,6 +569,13 @@ public class SqlParserTest {
   private static final ThreadLocal<boolean[]> LINUXIFY =
       ThreadLocal.withInitial(() -> new boolean[] {true});
 
+  private static final SqlWriterConfig SQL_WRITER_CONFIG =
+      SqlPrettyWriter.config()
+          .withAlwaysUseParentheses(true)
+          .withUpdateSetListNewline(false)
+          .withFromFolding(SqlWriterConfig.LineFolding.TALL)
+          .withIndentation(0);
+
   Quoting quoting = Quoting.DOUBLE_QUOTE;
   Casing unquotedCasing = Casing.TO_UPPER;
   Casing quotedCasing = Casing.UNCHANGED;
@@ -3768,10 +3778,10 @@ public class SqlParserTest {
 
   @Test public void testUpdateExtendedColumnList() {
     final String expected = "UPDATE `EMPDEFAULTS` EXTEND (`EXTRA` BOOLEAN, `NOTE` VARCHAR)"
-        + " SET `DEPTNO` = 1\n"
-        + ", `EXTRA` = TRUE\n"
-        + ", `EMPNO` = 20\n"
-        + ", `ENAME` = 'Bob'\n"
+        + " SET `DEPTNO` = 1"
+        + ", `EXTRA` = TRUE"
+        + ", `EMPNO` = 20"
+        + ", `ENAME` = 'Bob'"
         + ", `NOTE` = 'legion'\n"
         + "WHERE (`DEPTNO` = 10)";
     sql("update empdefaults(extra BOOLEAN, note VARCHAR)"
@@ -3783,10 +3793,10 @@ public class SqlParserTest {
 
   @Test public void testUpdateCaseSensitiveExtendedColumnList() {
     final String expected = "UPDATE `EMPDEFAULTS` EXTEND (`extra` BOOLEAN, `NOTE` VARCHAR)"
-        + " SET `DEPTNO` = 1\n"
-        + ", `extra` = TRUE\n"
-        + ", `EMPNO` = 20\n"
-        + ", `ENAME` = 'Bob'\n"
+        + " SET `DEPTNO` = 1"
+        + ", `extra` = TRUE"
+        + ", `EMPNO` = 20"
+        + ", `ENAME` = 'Bob'"
         + ", `NOTE` = 'legion'\n"
         + "WHERE (`DEPTNO` = 10)";
     sql("update empdefaults(\"extra\" BOOLEAN, note VARCHAR)"
@@ -3866,7 +3876,7 @@ public class SqlParserTest {
 
   @Test public void testUpdate() {
     sql("update emps set empno = empno + 1, sal = sal - 1 where empno=12")
-        .ok("UPDATE `EMPS` SET `EMPNO` = (`EMPNO` + 1)\n"
+        .ok("UPDATE `EMPS` SET `EMPNO` = (`EMPNO` + 1)"
             + ", `SAL` = (`SAL` - 1)\n"
             + "WHERE (`EMPNO` = 12)");
   }
@@ -3884,8 +3894,8 @@ public class SqlParserTest {
         + "FROM `TEMPEMPS`\n"
         + "WHERE (`DEPTNO` IS NULL)) AS `T`\n"
         + "ON (`E`.`EMPNO` = `T`.`EMPNO`)\n"
-        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`\n"
-        + ", `DEPTNO` = `T`.`DEPTNO`\n"
+        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`"
+        + ", `DEPTNO` = `T`.`DEPTNO`"
         + ", `SALARY` = (`T`.`SALARY` * 0.1)\n"
         + "WHEN NOT MATCHED THEN INSERT (`NAME`, `DEPT`, `SALARY`) "
         + "(VALUES (ROW(`T`.`NAME`, 10, (`T`.`SALARY` * 0.15))))";
@@ -3907,8 +3917,8 @@ public class SqlParserTest {
         + "FROM `TEMPEMPS`\n"
         + "WHERE (`DEPTNO` IS NULL)) AS `T`\n"
         + "ON (`E`.`EMPNO` = `T`.`EMPNO`)\n"
-        + "WHEN MATCHED THEN UPDATE SET `E`.`NAME` = `T`.`NAME`\n"
-        + ", `E`.`DEPTNO` = `T`.`DEPTNO`\n"
+        + "WHEN MATCHED THEN UPDATE SET `E`.`NAME` = `T`.`NAME`"
+        + ", `E`.`DEPTNO` = `T`.`DEPTNO`"
         + ", `E`.`SALARY` = (`T`.`SALARY` * 0.1)\n"
         + "WHEN NOT MATCHED THEN INSERT (`NAME`, `DEPT`, `SALARY`) "
         + "(VALUES (ROW(`T`.`NAME`, 10, (`T`.`SALARY` * 0.15))))";
@@ -3927,8 +3937,8 @@ public class SqlParserTest {
     final String expected = "MERGE INTO `EMPS` AS `E`\n"
         + "USING `TEMPEMPS` AS `T`\n"
         + "ON (`E`.`EMPNO` = `T`.`EMPNO`)\n"
-        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`\n"
-        + ", `DEPTNO` = `T`.`DEPTNO`\n"
+        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`"
+        + ", `DEPTNO` = `T`.`DEPTNO`"
         + ", `SALARY` = (`T`.`SALARY` * 0.1)\n"
         + "WHEN NOT MATCHED THEN INSERT (`NAME`, `DEPT`, `SALARY`) "
         + "(VALUES (ROW(`T`.`NAME`, 10, (`T`.`SALARY` * 0.15))))";
@@ -3947,8 +3957,8 @@ public class SqlParserTest {
     final String expected = "MERGE INTO `EMPS` AS `E`\n"
         + "USING `TEMPEMPS` AS `T`\n"
         + "ON (`E`.`EMPNO` = `T`.`EMPNO`)\n"
-        + "WHEN MATCHED THEN UPDATE SET `E`.`NAME` = `T`.`NAME`\n"
-        + ", `E`.`DEPTNO` = `T`.`DEPTNO`\n"
+        + "WHEN MATCHED THEN UPDATE SET `E`.`NAME` = `T`.`NAME`"
+        + ", `E`.`DEPTNO` = `T`.`DEPTNO`"
         + ", `E`.`SALARY` = (`T`.`SALARY` * 0.1)\n"
         + "WHEN NOT MATCHED THEN INSERT (`NAME`, `DEPT`, `SALARY`) "
         + "(VALUES (ROW(`T`.`NAME`, 10, (`T`.`SALARY` * 0.15))))";
@@ -7461,11 +7471,11 @@ public class SqlParserTest {
     SqlNode node = getSqlParser("alter system set schema = true").parseStmt();
     SqlSetOption opt = (SqlSetOption) node;
     assertThat(opt.getScope(), equalTo("SYSTEM"));
-    SqlPrettyWriter writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+    SqlPrettyWriter writer = new SqlPrettyWriter();
     assertThat(writer.format(opt.getName()), equalTo("\"SCHEMA\""));
-    writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+    writer = new SqlPrettyWriter();
     assertThat(writer.format(opt.getValue()), equalTo("TRUE"));
-    writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+    writer = new SqlPrettyWriter();
     assertThat(writer.format(opt),
         equalTo("ALTER SYSTEM SET \"SCHEMA\" = TRUE"));
 
@@ -7493,10 +7503,10 @@ public class SqlParserTest {
     node = getSqlParser("reset schema").parseStmt();
     opt = (SqlSetOption) node;
     assertThat(opt.getScope(), equalTo(null));
-    writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+    writer = new SqlPrettyWriter();
     assertThat(writer.format(opt.getName()), equalTo("\"SCHEMA\""));
     assertThat(opt.getValue(), equalTo(null));
-    writer = new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+    writer = new SqlPrettyWriter();
     assertThat(writer.format(opt),
         equalTo("RESET \"SCHEMA\""));
 
@@ -8716,7 +8726,7 @@ public class SqlParserTest {
     final String expected = "UPDATE `EMPS`\n"
         + "/*+ `PROPERTIES`(`K1` ='v1', `K2` ='v2'), "
         + "`INDEX`(`IDX1`, `IDX2`), `NO_HASH_JOIN` */ "
-        + "SET `EMPNO` = (`EMPNO` + 1)\n"
+        + "SET `EMPNO` = (`EMPNO` + 1)"
         + ", `SAL` = (`SAL` - 1)\n"
         + "WHERE (`EMPNO` = 12)";
     sql(sql).ok(expected);
@@ -8737,8 +8747,8 @@ public class SqlParserTest {
         + "AS `E`\n"
         + "USING `TEMPEMPS` AS `T`\n"
         + "ON (`E`.`EMPNO` = `T`.`EMPNO`)\n"
-        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`\n"
-        + ", `DEPTNO` = `T`.`DEPTNO`\n"
+        + "WHEN MATCHED THEN UPDATE SET `NAME` = `T`.`NAME`"
+        + ", `DEPTNO` = `T`.`DEPTNO`"
         + ", `SALARY` = (`T`.`SALARY` * 0.1)\n"
         + "WHEN NOT MATCHED THEN INSERT (`NAME`, `DEPT`, `SALARY`) "
         + "(VALUES (ROW(`T`.`NAME`, 10, (`T`.`SALARY` * 0.15))))";
@@ -8796,8 +8806,9 @@ public class SqlParserTest {
         SqlNode sqlNode,
         SqlDialect dialect,
         String expected) {
-      // no dialect, always parenthesize
-      final String actual = sqlNode.toSqlString(dialect, true).getSql();
+      final SqlDialect dialect2 = Util.first(dialect, AnsiSqlDialect.DEFAULT);
+      final SqlWriterConfig c2 = SQL_WRITER_CONFIG.withDialect(dialect2);
+      final String actual = sqlNode.toSqlString(c -> c2).getSql();
       TestUtil.assertEqualsVerbose(expected, linux(actual));
     }
 
@@ -8933,21 +8944,64 @@ public class SqlParserTest {
    * unparsing a query are consistent with the original query.
    */
   public class UnparsingTesterImpl extends TesterImpl {
+    private UnaryOperator<SqlWriterConfig> simple() {
+      return c -> c.withSelectListItemsOnSeparateLines(false)
+          .withUpdateSetListNewline(false)
+          .withIndentation(0)
+          .withFromFolding(SqlWriterConfig.LineFolding.TALL);
+    }
+
+    private UnaryOperator<SqlWriterConfig> simpleWithParens() {
+      return simple().andThen(withParens())::apply;
+    }
+
+    private UnaryOperator<SqlWriterConfig> simpleWithParensAnsi() {
+      return simpleWithParens().andThen(withAnsi())::apply;
+    }
+
+    private UnaryOperator<SqlWriterConfig> withParens() {
+      return c -> c.withAlwaysUseParentheses(true);
+    }
+
+    private UnaryOperator<SqlWriterConfig> withAnsi() {
+      return c -> c.withDialect(AnsiSqlDialect.DEFAULT);
+    }
+
+    private UnaryOperator<SqlWriterConfig> randomize(Random random) {
+      return c -> c.withFoldLength(random.nextInt(5) * 20 + 3)
+          .withHavingFolding(nextLineFolding(random))
+          .withWhereFolding(nextLineFolding(random))
+          .withSelectFolding(nextLineFolding(random))
+          .withFromFolding(nextLineFolding(random))
+          .withGroupByFolding(nextLineFolding(random))
+          .withClauseStartsLine(random.nextBoolean())
+          .withClauseEndsLine(random.nextBoolean());
+    }
+
+    private String toSqlString(SqlNodeList sqlNodeList,
+        UnaryOperator<SqlWriterConfig> transform) {
+      return sqlNodeList.getList().stream()
+          .map(node -> node.toSqlString(transform).getSql())
+          .collect(Collectors.joining(";"));
+    }
 
-    private String toSqlString(SqlNodeList sqlNodeList) {
-      List<String> sqls = sqlNodeList.getList().stream()
-          .map(it -> it.toSqlString(CalciteSqlDialect.DEFAULT, false).getSql())
-          .collect(Collectors.toList());
-      return String.join(";", sqls);
+    private SqlWriterConfig.LineFolding nextLineFolding(Random random) {
+      return nextEnum(random, SqlWriterConfig.LineFolding.class);
+    }
+
+    private <E extends Enum<E>> E nextEnum(Random random, Class<E> enumClass) {
+      final E[] constants = enumClass.getEnumConstants();
+      return constants[random.nextInt(constants.length)];
     }
 
     private void checkList(SqlNodeList sqlNodeList, List<String> expected) {
-      assertEquals(expected.size(), sqlNodeList.size());
+      assertThat(sqlNodeList.size(), is(expected.size()));
 
       for (int i = 0; i < sqlNodeList.size(); i++) {
         SqlNode sqlNode = sqlNodeList.get(i);
         // Unparse with no dialect, always parenthesize.
-        final String actual = sqlNode.toSqlString(null, true).getSql();
+        final String actual =
+            sqlNode.toSqlString(simpleWithParensAnsi()).getSql();
         assertEquals(expected.get(i), linux(actual));
       }
     }
@@ -8959,7 +9013,7 @@ public class SqlParserTest {
 
       // Unparse again in Calcite dialect (which we can parse), and
       // minimal parentheses.
-      final String sql1 = toSqlString(sqlNodeList);
+      final String sql1 = toSqlString(sqlNodeList, simple());
 
       // Parse and unparse again.
       SqlNodeList sqlNodeList2;
@@ -8970,7 +9024,7 @@ public class SqlParserTest {
       } finally {
         quoting = q;
       }
-      final String sql2 = toSqlString(sqlNodeList2);
+      final String sql2 = toSqlString(sqlNodeList2, simple());
 
       // Should be the same as we started with.
       assertEquals(sql1, sql2);
@@ -8979,6 +9033,10 @@ public class SqlParserTest {
       // If the unparser is not including sufficient parens to override
       // precedence, the problem will show up here.
       checkList(sqlNodeList2, expected);
+
+      final Random random = new Random();
+      final String sql3 = toSqlString(sqlNodeList, randomize(random));
+      assertThat(sql3, notNullValue());
     }
 
     @Override public void check(String sql, SqlDialect dialect, String expected,
@@ -8988,13 +9046,15 @@ public class SqlParserTest {
           parserChecker);
 
       // Unparse with the given dialect, always parenthesize.
-      final String actual = sqlNode.toSqlString(dialect, true).getSql();
+      final SqlDialect dialect2 = Util.first(dialect, AnsiSqlDialect.DEFAULT);
+      final UnaryOperator<SqlWriterConfig> transform =
+          simpleWithParens().andThen(c -> c.withDialect(dialect2))::apply;
+      final String actual = sqlNode.toSqlString(transform).getSql();
       assertEquals(expected, linux(actual));
 
       // Unparse again in Calcite dialect (which we can parse), and
       // minimal parentheses.
-      final String sql1 =
-          sqlNode.toSqlString(CalciteSqlDialect.DEFAULT, false).getSql();
+      final String sql1 = sqlNode.toSqlString(simple()).getSql();
 
       // Parse and unparse again.
       SqlNode sqlNode2;
@@ -9005,8 +9065,7 @@ public class SqlParserTest {
       } finally {
         quoting = q;
       }
-      final String sql2 =
-          sqlNode2.toSqlString(CalciteSqlDialect.DEFAULT, false).getSql();
+      final String sql2 = sqlNode2.toSqlString(simple()).getSql();
 
       // Should be the same as we started with.
       assertEquals(sql1, sql2);
@@ -9014,8 +9073,23 @@ public class SqlParserTest {
       // Now unparse again in the given dialect.
       // If the unparser is not including sufficient parens to override
       // precedence, the problem will show up here.
-      final String actual2 = sqlNode2.toSqlString(dialect, true).getSql();
+      final String actual2 = sqlNode.toSqlString(transform).getSql();
       assertEquals(expected, linux(actual2));
+
+      // Now unparse with a randomly configured SqlPrettyWriter.
+      // (This is a much a test for SqlPrettyWriter as for the parser.)
+      final Random random = new Random();
+      final String sql3 = sqlNode.toSqlString(randomize(random)).getSql();
+      assertThat(sql3, notNullValue());
+      SqlNode sqlNode4;
+      try {
+        quoting = Quoting.DOUBLE_QUOTE;
+        sqlNode4 = parseStmtAndHandleEx(sql1, b -> b, parser -> { });
+      } finally {
+        quoting = q;
+      }
+      final String sql4 = sqlNode4.toSqlString(simple()).getSql();
+      assertEquals(sql1, sql4);
     }
 
     @Override public void checkExp(String sql, String expected,
@@ -9023,13 +9097,15 @@ public class SqlParserTest {
       SqlNode sqlNode = parseExpressionAndHandleEx(sql, parserChecker);
 
       // Unparse with no dialect, always parenthesize.
-      final String actual = sqlNode.toSqlString(null, true).getSql();
+      final UnaryOperator<SqlWriterConfig> transform = c ->
+          simpleWithParens().apply(c).withDialect(AnsiSqlDialect.DEFAULT);
+      final String actual = sqlNode.toSqlString(transform).getSql();
       assertEquals(expected, linux(actual));
 
       // Unparse again in Calcite dialect (which we can parse), and
       // minimal parentheses.
       final String sql1 =
-          sqlNode.toSqlString(CalciteSqlDialect.DEFAULT, false).getSql();
+          sqlNode.toSqlString(UnaryOperator.identity()).getSql();
 
       // Parse and unparse again.
       // (Turn off parser checking, and use double-quotes.)
@@ -9042,7 +9118,7 @@ public class SqlParserTest {
         quoting = q;
       }
       final String sql2 =
-          sqlNode2.toSqlString(CalciteSqlDialect.DEFAULT, false).getSql();
+          sqlNode2.toSqlString(UnaryOperator.identity()).getSql();
 
       // Should be the same as we started with.
       assertEquals(sql1, sql2);
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 6559238..4300ba4 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -36,7 +36,6 @@ import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlOperandCountRange;
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.dialect.AnsiSqlDialect;
-import org.apache.calcite.sql.dialect.CalciteSqlDialect;
 import org.apache.calcite.sql.fun.SqlLibrary;
 import org.apache.calcite.sql.fun.SqlLibraryOperatorTableFactory;
 import org.apache.calcite.sql.fun.SqlLibraryOperators;
@@ -9004,8 +9003,7 @@ public abstract class SqlOperatorBaseTest {
           if (!typeChecker.checkOperandTypes(binding, false)) {
             continue;
           }
-          final SqlPrettyWriter writer =
-              new SqlPrettyWriter(CalciteSqlDialect.DEFAULT);
+          final SqlPrettyWriter writer = new SqlPrettyWriter();
           op.unparse(writer, call, 0, 0);
           final String s = writer.toSqlString().toString();
           if (s.startsWith("OVERLAY(")
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlPrettyWriterTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlPrettyWriterTest.java
index 57137b6..ebe61b2 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlPrettyWriterTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlPrettyWriterTest.java
@@ -19,6 +19,7 @@ package org.apache.calcite.sql.test;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.dialect.AnsiSqlDialect;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParser;
@@ -32,7 +33,6 @@ import org.junit.jupiter.api.Test;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.Objects;
-import java.util.function.Consumer;
 import java.util.function.UnaryOperator;
 
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -69,10 +69,10 @@ public class SqlPrettyWriterTest {
     private final boolean expr;
     private final String desc;
     private final String formatted;
-    private final UnaryOperator<SqlPrettyWriter> transform;
+    private final UnaryOperator<SqlWriterConfig> transform;
 
     Sql(String sql, boolean expr, String desc, String formatted,
-        UnaryOperator<SqlPrettyWriter> transform) {
+        UnaryOperator<SqlWriterConfig> transform) {
       this.sql = Objects.requireNonNull(sql);
       this.expr = expr;
       this.desc = desc;
@@ -80,13 +80,10 @@ public class SqlPrettyWriterTest {
       this.transform = Objects.requireNonNull(transform);
     }
 
-    Sql withWriter(Consumer<SqlPrettyWriter> consumer) {
-      Objects.requireNonNull(consumer);
-      return new Sql(sql, expr, desc, formatted, w -> {
-        final SqlPrettyWriter w2 = transform.apply(w);
-        consumer.accept(w2);
-        return w2;
-      });
+    Sql withWriter(UnaryOperator<SqlWriterConfig> transform) {
+      Objects.requireNonNull(transform);
+      return new Sql(sql, expr, desc, formatted, w ->
+          transform.apply(this.transform.apply(w)));
     }
 
     Sql expectingDesc(String desc) {
@@ -108,8 +105,10 @@ public class SqlPrettyWriterTest {
     }
 
     Sql check() {
-      final SqlPrettyWriter prettyWriter =
-          transform.apply(new SqlPrettyWriter(AnsiSqlDialect.DEFAULT));
+      final SqlWriterConfig config =
+          transform.apply(SqlPrettyWriter.config()
+              .withDialect(AnsiSqlDialect.DEFAULT));
+      final SqlPrettyWriter prettyWriter = new SqlPrettyWriter(config);
       final SqlNode node;
       if (expr) {
         final SqlCall valuesCall = (SqlCall) parseQuery("VALUES (" + sql + ")");
@@ -169,11 +168,11 @@ public class SqlPrettyWriterTest {
   }
 
   private Sql sql(String sql) {
-    return new Sql(sql, false, "${desc}", "${formatted}", w -> w);
+    return new Sql(sql, false, null, "${formatted}", w -> w);
   }
 
   private Sql expr(String sql) {
-    return sql(sql).withExpr(true).expectingDesc(null);
+    return sql(sql).withExpr(true);
   }
 
   // ~ Tests ----------------------------------------------------------------
@@ -184,44 +183,113 @@ public class SqlPrettyWriterTest {
 
   @Test public void testIndent8() {
     simple()
-        .withWriter(w -> w.setIndentation(8))
+        .expectingDesc("${desc}")
+        .withWriter(w -> w.withIndentation(8))
         .check();
   }
 
   @Test public void testClausesNotOnNewLine() {
     simple()
-        .withWriter(w -> w.setClauseStartsLine(false))
+        .withWriter(w -> w.withClauseStartsLine(false))
         .check();
   }
 
   @Test public void testSelectListItemsOnSeparateLines() {
     simple()
-        .withWriter(w -> w.setSelectListItemsOnSeparateLines(true))
+        .withWriter(w -> w.withSelectListItemsOnSeparateLines(true))
         .check();
   }
 
-  @Test public void testSelectListExtraIndentFlag() {
+  @Test public void testSelectListNoExtraIndentFlag() {
     simple()
-        .withWriter(w -> w.setSelectListItemsOnSeparateLines(true))
-        .withWriter(w -> w.setSelectListExtraIndentFlag(false))
+        .withWriter(w -> w.withSelectListItemsOnSeparateLines(true)
+            .withSelectListExtraIndentFlag(false)
+            .withClauseEndsLine(true))
+        .check();
+  }
+
+  @Test public void testFold() {
+    simple()
+        .withWriter(w -> w.withLineFolding(SqlWriterConfig.LineFolding.FOLD)
+            .withFoldLength(45))
+        .check();
+  }
+
+  @Test public void testChop() {
+    simple()
+        .withWriter(w -> w.withLineFolding(SqlWriterConfig.LineFolding.CHOP)
+            .withFoldLength(45))
+        .check();
+  }
+
+  @Test public void testChopLeadingComma() {
+    simple()
+        .withWriter(w -> w.withLineFolding(SqlWriterConfig.LineFolding.CHOP)
+            .withFoldLength(45)
+            .withLeadingComma(true))
+        .check();
+  }
+
+  @Test public void testLeadingComma() {
+    simple()
+        .withWriter(w -> w.withLeadingComma(true)
+            .withSelectListItemsOnSeparateLines(true)
+            .withSelectListExtraIndentFlag(true))
+        .check();
+  }
+
+  @Test public void testClauseEndsLine() {
+    simple()
+        .withWriter(w -> w.withClauseEndsLine(true)
+            .withLineFolding(SqlWriterConfig.LineFolding.WIDE)
+            .withFoldLength(45))
+        .check();
+  }
+
+  @Test public void testClauseEndsLineTall() {
+    simple()
+        .withWriter(w -> w.withClauseEndsLine(true)
+            .withLineFolding(SqlWriterConfig.LineFolding.TALL)
+            .withFoldLength(45))
+        .check();
+  }
+
+  @Test public void testClauseEndsLineFold() {
+    simple()
+        .withWriter(w -> w.withClauseEndsLine(true)
+            .withLineFolding(SqlWriterConfig.LineFolding.FOLD)
+            .withFoldLength(45))
+        .check();
+  }
+
+  /** Tests formatting a query with Looker's preferences. */
+  @Test public void testLooker() {
+    simple()
+        .withWriter(w -> w.withFoldLength(60)
+            .withLineFolding(SqlWriterConfig.LineFolding.STEP)
+            .withSelectFolding(SqlWriterConfig.LineFolding.TALL)
+            .withFromFolding(SqlWriterConfig.LineFolding.TALL)
+            .withWhereFolding(SqlWriterConfig.LineFolding.TALL)
+            .withHavingFolding(SqlWriterConfig.LineFolding.TALL)
+            .withClauseEndsLine(true))
         .check();
   }
 
   @Test public void testKeywordsLowerCase() {
     simple()
-        .withWriter(w -> w.setKeywordsLowerCase(true))
+        .withWriter(w -> w.withKeywordsLowerCase(true))
         .check();
   }
 
   @Test public void testParenthesizeAllExprs() {
     simple()
-        .withWriter(w -> w.setAlwaysUseParentheses(true))
+        .withWriter(w -> w.withAlwaysUseParentheses(true))
         .check();
   }
 
   @Test public void testOnlyQuoteIdentifiersWhichNeedIt() {
     simple()
-        .withWriter(w -> w.setQuoteAllIdentifiers(false))
+        .withWriter(w -> w.withQuoteAllIdentifiers(false))
         .check();
   }
 
@@ -229,7 +297,27 @@ public class SqlPrettyWriterTest {
     // Note that ( is at the indent, SELECT is on the same line, and ) is
     // below it.
     simple()
-        .withWriter(w -> w.setSubQueryStyle(SqlWriter.SubQueryStyle.BLACK))
+        .withWriter(w -> w.withSubQueryStyle(SqlWriter.SubQueryStyle.BLACK))
+        .check();
+  }
+
+  @Test public void testBlackSubQueryStyleIndent0() {
+    simple()
+        .withWriter(w -> w.withSubQueryStyle(SqlWriter.SubQueryStyle.BLACK)
+            .withIndentation(0))
+        .check();
+  }
+
+  @Test public void testValuesNewline() {
+    sql("select * from (values (1, 2), (3, 4)) as t")
+        .withWriter(w -> w.withValuesListNewline(true))
+        .check();
+  }
+
+  @Test public void testValuesLeadingCommas() {
+    sql("select * from (values (1, 2), (3, 4)) as t")
+        .withWriter(w -> w.withValuesListNewline(true)
+            .withLeadingComma(true))
         .check();
   }
 
@@ -260,7 +348,7 @@ public class SqlPrettyWriterTest {
         + "ELSE 7\n"
         + "END";
     expr(sql)
-        .withWriter(w -> w.setCaseClausesOnNewLines(true))
+        .withWriter(w -> w.withCaseClausesOnNewLines(true))
         .expectingFormatted(formatted)
         .check();
   }
@@ -310,10 +398,6 @@ public class SqlPrettyWriterTest {
   }
 
   @Test public void testUnion() {
-    // todo: SELECT should not be indented from UNION, like this:
-    // UNION
-    //     SELECT *
-    //     FROM `W`
     final String sql = "select * from t "
         + "union select * from ("
         + "  select * from u "
@@ -321,19 +405,50 @@ public class SqlPrettyWriterTest {
         + "union select * from w "
         + "order by a, b";
     sql(sql)
-        .expectingDesc(null)
         .check();
   }
 
   @Test public void testMultiset() {
     sql("values (multiset (select * from t))")
-        .expectingDesc(null)
         .check();
   }
 
+  @Test public void testJoinComma() {
+    final String sql = "select *\n"
+        + "from x, y as y1, z, (select * from a, a2 as a3),\n"
+        + " (select * from b) as b2\n"
+        + "where p = q\n"
+        + "and exists (select 1 from v, w)";
+    sql(sql).check();
+  }
+
   @Test public void testInnerJoin() {
     sql("select * from x inner join y on x.k=y.k")
-        .expectingDesc(null)
+        .check();
+  }
+
+  @Test public void testJoinTall() {
+    sql("select * from x inner join y on x.k=y.k left join z using (a)")
+        .withWriter(c -> c.withLineFolding(SqlWriterConfig.LineFolding.TALL))
+        .check();
+  }
+
+  @Test public void testJoinTallClauseEndsLine() {
+    sql("select * from x inner join y on x.k=y.k left join z using (a)")
+        .withWriter(c -> c.withLineFolding(SqlWriterConfig.LineFolding.TALL)
+            .withClauseEndsLine(true))
+        .check();
+  }
+
+  @Test public void testJoinLateralSubQueryTall() {
+    final String sql = "select *\n"
+        + "from (select a from customers where b < c group by d) as c,\n"
+        + " products,\n"
+        + " lateral (select e from orders where exists (\n"
+        + "    select 1 from promotions)) as t5\n"
+        + "group by f";
+    sql(sql)
+        .withWriter(c -> c.withLineFolding(SqlWriterConfig.LineFolding.TALL))
         .check();
   }
 
@@ -344,9 +459,9 @@ public class SqlPrettyWriterTest {
         + " or ((a or b) is true) and d not in (f,g)"
         + " or x <> z";
     sql(sql)
-        .withWriter(w -> w.setSelectListItemsOnSeparateLines(true))
-        .withWriter(w -> w.setSelectListExtraIndentFlag(false))
-        .withWriter(w -> w.setWhereListItemsOnSeparateLines(true))
+        .withWriter(w -> w.withSelectListItemsOnSeparateLines(true)
+            .withSelectListExtraIndentFlag(false)
+            .withWhereListItemsOnSeparateLines(true))
         .check();
   }
 
@@ -357,9 +472,80 @@ public class SqlPrettyWriterTest {
         + " or ((a or b) is true)) and (d not in (f,g)"
         + " or v <> ((w * x) + y) * z)";
     sql(sql)
-        .withWriter(w -> w.setSelectListItemsOnSeparateLines(true))
-        .withWriter(w -> w.setSelectListExtraIndentFlag(false))
-        .withWriter(w -> w.setWhereListItemsOnSeparateLines(true))
+        .withWriter(w -> w.withSelectListItemsOnSeparateLines(true)
+            .withSelectListExtraIndentFlag(false)
+            .withWhereListItemsOnSeparateLines(true))
+        .check();
+  }
+
+  /** As {@link #testWhereListItemsOnSeparateLinesAnd()}, but
+   * with {@link SqlWriterConfig#clauseEndsLine ClauseEndsLine=true}. */
+  @Test public void testWhereListItemsOnSeparateLinesAndNewline() {
+    final String sql = "select x"
+        + " from y"
+        + " where h is not null and (i < j"
+        + " or ((a or b) is true)) and (d not in (f,g)"
+        + " or v <> ((w * x) + y) * z)";
+    sql(sql)
+        .withWriter(w -> w.withSelectListItemsOnSeparateLines(true)
+            .withSelectListExtraIndentFlag(false)
+            .withWhereListItemsOnSeparateLines(true)
+            .withClauseEndsLine(true))
+        .check();
+  }
+
+  @Test public void testUpdate() {
+    final String sql = "update emp\n"
+        + "set mgr = mgr + 1, deptno = 5\n"
+        + "where deptno = 10 and name = 'Fred'";
+    sql(sql)
         .check();
   }
+
+  @Test public void testUpdateNoLine() {
+    final String sql = "update emp\n"
+        + "set mgr = mgr + 1, deptno = 5\n"
+        + "where deptno = 10 and name = 'Fred'";
+    sql(sql)
+        .withWriter(w -> w.withUpdateSetListNewline(false))
+        .check();
+  }
+
+  @Test public void testUpdateNoLine2() {
+    final String sql = "update emp\n"
+        + "set mgr = mgr + 1, deptno = 5\n"
+        + "where deptno = 10 and name = 'Fred'";
+    sql(sql)
+        .withWriter(w -> w.withUpdateSetListNewline(false)
+            .withClauseStartsLine(false))
+        .check();
+  }
+
+  public static void main(String[] args) throws SqlParseException {
+    final String sql = "select x as a, b as b, c as c, d,"
+        + " 'mixed-Case string',"
+        + " unquotedCamelCaseId,"
+        + " \"quoted id\" "
+        + "from"
+        + " (select *"
+        + " from t"
+        + " where x = y and a > 5"
+        + " group by z, zz"
+        + " window w as (partition by c),"
+        + "  w1 as (partition by c,d order by a, b"
+        + "   range between interval '2:2' hour to minute preceding"
+        + "    and interval '1' day following)) "
+        + "order by gg desc nulls last, hh asc";
+    final SqlNode node = SqlParser.create(sql).parseQuery();
+
+    final SqlWriterConfig config = SqlPrettyWriter.config()
+        .withLineFolding(SqlWriterConfig.LineFolding.STEP)
+        .withSelectFolding(SqlWriterConfig.LineFolding.TALL)
+        .withFromFolding(SqlWriterConfig.LineFolding.TALL)
+        .withWhereFolding(SqlWriterConfig.LineFolding.TALL)
+        .withHavingFolding(SqlWriterConfig.LineFolding.TALL)
+        .withIndentation(4)
+        .withClauseEndsLine(true);
+    System.out.println(new SqlPrettyWriter(config).format(node));
+  }
 }
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java
index e4336fc..25f0c0c 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java
@@ -306,7 +306,7 @@ public class JdbcAdapterTest {
             + "\"t0\".\"DNAME\", \"SALGRADE\".\"GRADE\"\n"
             + "FROM \"SCOTT\".\"SALGRADE\"\n"
             + "INNER JOIN ((SELECT \"EMPNO\", \"ENAME\", \"SAL\", \"DEPTNO\"\n"
-            + "FROM \"SCOTT\".\"EMP\") AS \"t\"\n"
+            + "FROM \"SCOTT\".\"EMP\") AS \"t\" "
             + "INNER JOIN (SELECT \"DEPTNO\", \"DNAME\"\n"
             + "FROM \"SCOTT\".\"DEPT\") AS \"t0\" ON \"t\".\"DEPTNO\" = \"t0\".\"DEPTNO\")"
             + " ON \"SALGRADE\".\"LOSAL\" < \"t\".\"SAL\" AND \"SALGRADE\".\"HISAL\" > \"t\".\"SAL\"");
diff --git a/core/src/test/resources/org/apache/calcite/sql/test/SqlPrettyWriterTest.xml b/core/src/test/resources/org/apache/calcite/sql/test/SqlPrettyWriterTest.xml
index 3b81a8c..84a38b6 100644
--- a/core/src/test/resources/org/apache/calcite/sql/test/SqlPrettyWriterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/sql/test/SqlPrettyWriterTest.xml
@@ -16,8 +16,20 @@
   ~ limitations under the License.
   -->
 <Root>
+  <TestCase name="testBlackSubQueryStyleIndent0">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
+FROM
+(SELECT *
+FROM `T`
+WHERE `X` = `Y` AND `A` > 5
+GROUP BY `Z`, `ZZ`
+WINDOW `W` AS (PARTITION BY `C`),
+`W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
   <TestCase name="testDefault">
-    <Resource name="desc"/>
     <Resource name="formatted">
       <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
 FROM (SELECT *
@@ -44,6 +56,105 @@ ELSE 7
 END]]>
     </Resource>
   </TestCase>
+  <TestCase name="testChop">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`,
+    `B` AS `B`,
+    `C` AS `C`,
+    `D`,
+    'mixed-Case string',
+    `UNQUOTEDCAMELCASEID`,
+    `quoted id`
+FROM (SELECT *
+        FROM `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`, `ZZ`
+        WINDOW `W` AS (PARTITION BY `C`),
+            `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testChopLeadingComma">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`
+  , `B` AS `B`
+  , `C` AS `C`
+  , `D`
+  , 'mixed-Case string'
+  , `UNQUOTEDCAMELCASEID`
+  , `quoted id`
+FROM (SELECT *
+        FROM `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`, `ZZ`
+        WINDOW `W` AS (PARTITION BY `C`)
+          , `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testClauseEndsLine">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
+FROM
+    (SELECT *
+        FROM
+            `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`, `ZZ`
+        WINDOW `W` AS (PARTITION BY `C`), `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testClauseEndsLineFold">
+    <Resource name="formatted">
+      <![CDATA[SELECT
+    `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`,
+    'mixed-Case string',
+    `UNQUOTEDCAMELCASEID`, `quoted id`
+FROM
+    (SELECT *
+        FROM
+            `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`, `ZZ`
+        WINDOW
+            `W` AS (PARTITION BY `C`),
+            `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testClauseEndsLineTall">
+    <Resource name="formatted">
+      <![CDATA[SELECT
+    `X` AS `A`,
+    `B` AS `B`,
+    `C` AS `C`,
+    `D`,
+    'mixed-Case string',
+    `UNQUOTEDCAMELCASEID`,
+    `quoted id`
+FROM
+    (SELECT
+            *
+        FROM
+            `T`
+        WHERE
+            `X` = `Y`
+            AND `A` > 5
+        GROUP BY
+            `Z`,
+            `ZZ`
+        WINDOW
+            `W` AS (
+                PARTITION BY `C`),
+            `W1` AS (
+                PARTITION BY `C`, `D`
+                ORDER BY `A`, `B`
+                RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY
+    `GG`]]>
+    </Resource>
+  </TestCase>
   <TestCase name="testIndent8">
     <Resource name="desc">
       <![CDATA[indentation=8]]>
@@ -60,39 +171,45 @@ ORDER BY `GG`]]>
     </Resource>
   </TestCase>
   <TestCase name="testClausesNotOnNewLine">
-    <Resource name="desc"/>
     <Resource name="formatted">
-      <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id` FROM (SELECT *FROM `T` WHERE `X` = `Y` AND `A` > 5 GROUP BY `Z`, `ZZ` WINDOW `W` AS (PARTITION BY `C`),
+      <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id` FROM (SELECT *FROM `T` WHERE `X` = `Y` AND `A` > 5 GROUP BY `Z`, `ZZ` WINDOW `W` AS (PARTITION BY `C`), `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testFold">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`,
+    `D`, 'mixed-Case string',
+    `UNQUOTEDCAMELCASEID`, `quoted id`
+FROM (SELECT *
+        FROM `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`, `ZZ`
+        WINDOW `W` AS (PARTITION BY `C`),
             `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
 ORDER BY `GG`]]>
     </Resource>
   </TestCase>
   <TestCase name="testSelectListItemsOnSeparateLines">
-    <Resource name="desc"/>
     <Resource name="formatted">
-      <![CDATA[SELECT
-    `X` AS `A`,
-        `B` AS `B`,
-        `C` AS `C`,
+      <![CDATA[SELECT `X` AS `A`,
+    `B` AS `B`,
+    `C` AS `C`,
     `D`,
     'mixed-Case string',
     `UNQUOTEDCAMELCASEID`,
     `quoted id`
-FROM (SELECT
-            *
+FROM (SELECT *
         FROM `T`
         WHERE `X` = `Y` AND `A` > 5
-        GROUP BY
-            `Z`,
+        GROUP BY `Z`,
             `ZZ`
         WINDOW `W` AS (PARTITION BY `C`),
             `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
-ORDER BY
-`GG`]]>
+ORDER BY `GG`]]>
     </Resource>
   </TestCase>
-  <TestCase name="testSelectListExtraIndentFlag">
-    <Resource name="desc"/>
+  <TestCase name="testSelectListNoExtraIndentFlag">
     <Resource name="formatted">
       <![CDATA[SELECT
     `X` AS `A`,
@@ -102,21 +219,23 @@ ORDER BY
     'mixed-Case string',
     `UNQUOTEDCAMELCASEID`,
     `quoted id`
-FROM (SELECT
+FROM
+    (SELECT
             *
-        FROM `T`
+        FROM
+            `T`
         WHERE `X` = `Y` AND `A` > 5
         GROUP BY
             `Z`,
             `ZZ`
-        WINDOW `W` AS (PARTITION BY `C`),
+        WINDOW
+            `W` AS (PARTITION BY `C`),
             `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
 ORDER BY
-`GG`]]>
+    `GG`]]>
     </Resource>
   </TestCase>
   <TestCase name="testKeywordsLowerCase">
-    <Resource name="desc"/>
     <Resource name="formatted">
       <![CDATA[select `X` as `A`, `B` as `B`, `C` as `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
 from (select *
@@ -129,7 +248,6 @@ order by `GG`]]>
     </Resource>
   </TestCase>
   <TestCase name="testParenthesizeAllExprs">
-    <Resource name="desc"/>
     <Resource name="formatted">
       <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
 FROM (SELECT *
@@ -142,7 +260,6 @@ ORDER BY `GG`]]>
     </Resource>
   </TestCase>
   <TestCase name="testOnlyQuoteIdentifiersWhichNeedIt">
-    <Resource name="desc"/>
     <Resource name="formatted">
       <![CDATA[SELECT X AS A, B AS B, C AS C, D, 'mixed-Case string', UNQUOTEDCAMELCASEID, `quoted id`
 FROM (SELECT *
@@ -155,10 +272,10 @@ ORDER BY GG]]>
     </Resource>
   </TestCase>
   <TestCase name="testBlackSubQueryStyle">
-    <Resource name="desc"/>
     <Resource name="formatted">
       <![CDATA[SELECT `X` AS `A`, `B` AS `B`, `C` AS `C`, `D`, 'mixed-Case string', `UNQUOTEDCAMELCASEID`, `quoted id`
-FROM (    SELECT *
+FROM
+(   SELECT *
         FROM `T`
         WHERE `X` = `Y` AND `A` > 5
         GROUP BY `Z`, `ZZ`
@@ -182,6 +299,74 @@ ORDER BY `GG`]]>
       <![CDATA[CAST(`X` + `Y` AS DECIMAL(5, 10))]]>
     </Resource>
   </TestCase>
+  <TestCase name="testJoinComma">
+    <Resource name="formatted">
+      <![CDATA[SELECT *
+FROM `X`,
+    `Y` AS `Y1`,
+    `Z`,
+        (SELECT *
+        FROM `A`,
+            `A2` AS `A3`),
+        (SELECT *
+        FROM `B`) AS `B2`
+WHERE `P` = `Q` AND EXISTS (SELECT 1
+        FROM `V`,
+            `W`)]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinLateralSubQueryTall">
+    <Resource name="formatted">
+      <![CDATA[SELECT *
+FROM (SELECT `A`
+        FROM `CUSTOMERS`
+        WHERE `B` < `C`
+        GROUP BY `D`) AS `C`,
+    `PRODUCTS`,
+    LATERAL (SELECT `E`
+        FROM `ORDERS`
+        WHERE EXISTS (SELECT 1
+                FROM `PROMOTIONS`)) AS `T5`
+GROUP BY `F`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinTall">
+    <Resource name="formatted">
+      <![CDATA[SELECT *
+FROM `X`
+    INNER JOIN `Y` ON `X`.`K` = `Y`.`K`
+    LEFT JOIN `Z` USING (`A`)]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinTallClauseEndsLine">
+    <Resource name="formatted">
+      <![CDATA[SELECT
+    *
+FROM
+    `X`
+    INNER JOIN `Y` ON `X`.`K` = `Y`.`K`
+    LEFT JOIN `Z` USING (`A`)]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testLeadingComma">
+    <Resource name="formatted">
+      <![CDATA[SELECT `X` AS `A`
+  , `B` AS `B`
+  , `C` AS `C`
+  , `D`
+  , 'mixed-Case string'
+  , `UNQUOTEDCAMELCASEID`
+  , `quoted id`
+FROM (SELECT *
+        FROM `T`
+        WHERE `X` = `Y` AND `A` > 5
+        GROUP BY `Z`
+          , `ZZ`
+        WINDOW `W` AS (PARTITION BY `C`)
+          , `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY `GG`]]>
+    </Resource>
+  </TestCase>
   <TestCase name="testLiteralChain">
     <Resource name="formatted">
       <![CDATA['x'
@@ -189,25 +374,52 @@ ORDER BY `GG`]]>
 'z']]>
     </Resource>
   </TestCase>
+  <TestCase name="testLooker">
+    <Resource name="formatted">
+      <![CDATA[SELECT
+    `X` AS `A`,
+    `B` AS `B`,
+    `C` AS `C`,
+    `D`,
+    'mixed-Case string',
+    `UNQUOTEDCAMELCASEID`,
+    `quoted id`
+FROM
+    (SELECT
+            *
+        FROM
+            `T`
+        WHERE
+            `X` = `Y`
+            AND `A` > 5
+        GROUP BY
+            `Z`, `ZZ`
+        WINDOW
+            `W` AS (PARTITION BY `C`),
+            `W1` AS (PARTITION BY `C`, `D` ORDER BY `A`, `B` RANGE BETWEEN INTERVAL '2:2' HOUR TO MINUTE PRECEDING AND INTERVAL '1' DAY FOLLOWING))
+ORDER BY
+    `GG`]]>
+    </Resource>
+  </TestCase>
   <TestCase name="testOverlaps">
     <Resource name="formatted">
-      <![CDATA[(`X`, `XX`) OVERLAPS (`Y`, `YY`) OR `X` IS NOT NULL]]>
+      <![CDATA[PERIOD (`X`, `XX`) OVERLAPS PERIOD (`Y`, `YY`) OR `X` IS NOT NULL]]>
     </Resource>
   </TestCase>
   <TestCase name="testUnion">
     <Resource name="formatted">
       <![CDATA[SELECT *
-        FROM `T`
-    UNION
-        SELECT *
-        FROM (SELECT *
-                        FROM `U`
-                    UNION
-                        SELECT *
-                        FROM `V`)
+FROM `T`
+UNION
+SELECT *
+FROM (SELECT *
+            FROM `U`
+            UNION
+            SELECT *
+            FROM `V`)
 UNION
-    SELECT *
-    FROM `W`
+SELECT *
+FROM `W`
 ORDER BY `A`, `B`]]>
     </Resource>
   </TestCase>
@@ -224,11 +436,42 @@ FROM `X`
     INNER JOIN `Y` ON `X`.`K` = `Y`.`K`]]>
     </Resource>
   </TestCase>
+  <TestCase name="testUpdate">
+    <Resource name="formatted">
+      <![CDATA[UPDATE `EMP` SET
+    `MGR` = `MGR` + 1,
+    `DEPTNO` = 5
+WHERE `DEPTNO` = 10 AND `NAME` = 'Fred']]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testUpdateNoLine">
+    <Resource name="formatted">
+      <![CDATA[UPDATE `EMP` SET `MGR` = `MGR` + 1, `DEPTNO` = 5
+WHERE `DEPTNO` = 10 AND `NAME` = 'Fred']]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testUpdateNoLine2">
+    <Resource name="formatted">
+      <![CDATA[UPDATE `EMP` SET `MGR` = `MGR` + 1, `DEPTNO` = 5 WHERE `DEPTNO` = 10 AND `NAME` = 'Fred']]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testValuesLeadingCommas">
+    <Resource name="formatted">
+      <![CDATA[SELECT *
+FROM (VALUES ROW(1, 2)
+          , ROW(3, 4)) AS `T`]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testValuesNewline">
+    <Resource name="formatted">
+      <![CDATA[SELECT *
+FROM (VALUES ROW(1, 2),
+            ROW(3, 4)) AS `T`]]>
+    </Resource>
+  </TestCase>
   <TestCase name="testWhereListItemsOnSeparateLinesOr">
-    <Resource name="desc"/>
     <Resource name="formatted">
-      <![CDATA[SELECT
-    `X`
+      <![CDATA[SELECT `X`
 FROM `Y`
 WHERE `H` IS NOT NULL AND `I` < `J`
     OR (`A` OR `B`) IS TRUE AND `D` NOT IN (`F`, `G`)
@@ -236,14 +479,24 @@ WHERE `H` IS NOT NULL AND `I` < `J`
     </Resource>
   </TestCase>
   <TestCase name="testWhereListItemsOnSeparateLinesAnd">
-    <Resource name="desc"/>
     <Resource name="formatted">
-      <![CDATA[SELECT
-    `X`
+      <![CDATA[SELECT `X`
 FROM `Y`
 WHERE `H` IS NOT NULL
     AND (`I` < `J` OR (`A` OR `B`) IS TRUE)
     AND (`D` NOT IN (`F`, `G`) OR `V` <> (`W` * `X` + `Y`) * `Z`)]]>
     </Resource>
   </TestCase>
+  <TestCase name="testWhereListItemsOnSeparateLinesAndNewline">
+    <Resource name="formatted">
+      <![CDATA[SELECT
+    `X`
+FROM
+    `Y`
+WHERE
+    `H` IS NOT NULL
+    AND (`I` < `J` OR (`A` OR `B`) IS TRUE)
+    AND (`D` NOT IN (`F`, `G`) OR `V` <> (`W` * `X` + `Y`) * `Z`)]]>
+    </Resource>
+  </TestCase>
 </Root>
diff --git a/piglet/src/main/java/org/apache/calcite/piglet/PigConverter.java b/piglet/src/main/java/org/apache/calcite/piglet/PigConverter.java
index a7a8e63..2db0bb7 100644
--- a/piglet/src/main/java/org/apache/calcite/piglet/PigConverter.java
+++ b/piglet/src/main/java/org/apache/calcite/piglet/PigConverter.java
@@ -35,6 +35,7 @@ import org.apache.calcite.rel.rules.ProjectWindowTransposeRule;
 import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.pretty.SqlPrettyWriter;
 import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.Program;
@@ -221,11 +222,13 @@ public class PigConverter extends PigServer {
    */
   public List<String> pigToSql(String pigQuery, SqlDialect sqlDialect)
       throws IOException {
-    final SqlPrettyWriter writer = new SqlPrettyWriter(sqlDialect);
-    writer.setQuoteAllIdentifiers(false);
-    writer.setAlwaysUseParentheses(false);
-    writer.setSelectListItemsOnSeparateLines(false);
-    writer.setIndentation(2);
+    final SqlWriterConfig config = SqlPrettyWriter.config()
+        .withQuoteAllIdentifiers(false)
+        .withAlwaysUseParentheses(false)
+        .withSelectListItemsOnSeparateLines(false)
+        .withIndentation(2)
+        .withDialect(sqlDialect);
+    final SqlPrettyWriter writer = new SqlPrettyWriter(config);
     return pigToSql(pigQuery, writer);
   }
 
diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
index 4098989..feb6020 100644
--- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
+++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
@@ -253,12 +253,12 @@ public class PigRelOpTest extends PigRelTestBase {
 
     final String sql = ""
         + "SELECT *\n"
-        + "  FROM scott.EMP\n"
-        + "  WHERE DEPTNO = 10\n"
+        + "FROM scott.EMP\n"
+        + "WHERE DEPTNO = 10\n"
         + "UNION ALL\n"
-        + "  SELECT *\n"
-        + "  FROM scott.EMP\n"
-        + "  WHERE DEPTNO = 20";
+        + "SELECT *\n"
+        + "FROM scott.EMP\n"
+        + "WHERE DEPTNO = 20";
     pig(script)
         .assertRel("B1", false,
             hasTree("LogicalFilter(condition=[=($7, 10)])\n"
@@ -377,11 +377,11 @@ public class PigRelOpTest extends PigRelTestBase {
         + " $cor1.SAL, $cor1.COMM, $cor1.DEPTNO0 AS DEPTNO\n"
         + "FROM (SELECT DEPTNO, JOB, COLLECT(ROW(EMPNO, ENAME, JOB, MGR, "
         + "HIREDATE, SAL, COMM, DEPTNO)) AS $f2\n"
-        + "      FROM scott.EMP\n"
-        + "      WHERE JOB <> 'CLERK'\n"
-        + "      GROUP BY DEPTNO, JOB) AS $cor1,\n"
-        + "    LATERAL UNNEST (SELECT $cor1.$f2 AS $f0\n"
-        + "        FROM (VALUES  (0)) AS t (ZERO)) AS t3 (EMPNO, ENAME, JOB,"
+        + "    FROM scott.EMP\n"
+        + "    WHERE JOB <> 'CLERK'\n"
+        + "    GROUP BY DEPTNO, JOB) AS $cor1,\n"
+        + "  LATERAL UNNEST (SELECT $cor1.$f2 AS $f0\n"
+        + "    FROM (VALUES  (0)) AS t (ZERO)) AS t3 (EMPNO, ENAME, JOB,"
         + " MGR, HIREDATE, SAL, COMM, DEPTNO) AS t30\n"
         + "ORDER BY $cor1.DEPTNO, $cor1.JOB";
     pig(script).assertRel(hasTree(plan))
@@ -464,20 +464,20 @@ public class PigRelOpTest extends PigRelTestBase {
         + "FROM (SELECT $cor4.DEPTNO AS group, "
         + "COUNT(PIG_BAG($cor4.X)) AS cnt, $cor4.X, "
         + "BigDecimalMax(PIG_BAG(MULTISET_PROJECTION($cor4.X, 3))) AS $f3\n"
-        + "      FROM (SELECT DEPTNO, COLLECT(ROW(EMPNO, ENAME, JOB, MGR, "
+        + "    FROM (SELECT DEPTNO, COLLECT(ROW(EMPNO, ENAME, JOB, MGR, "
         + "HIREDATE, SAL, COMM, DEPTNO)) AS A\n"
-        + "            FROM scott.EMP\n"
-        + "            GROUP BY DEPTNO) AS $cor4,\n"
-        + "          LATERAL (SELECT COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n"
-        + "            FROM (SELECT ENAME, JOB, DEPTNO, SAL\n"
-        + "                  FROM UNNEST (SELECT $cor4.A AS $f0\n"
-        + "                        FROM (VALUES  (0)) AS t (ZERO)) "
+        + "        FROM scott.EMP\n"
+        + "        GROUP BY DEPTNO) AS $cor4,\n"
+        + "      LATERAL (SELECT COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n"
+        + "        FROM (SELECT ENAME, JOB, DEPTNO, SAL\n"
+        + "            FROM UNNEST (SELECT $cor4.A AS $f0\n"
+        + "                FROM (VALUES  (0)) AS t (ZERO)) "
         + "AS t2 (EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, DEPTNO)\n"
-        + "                  WHERE JOB <> 'CLERK'\n"
-        + "                  ORDER BY SAL) AS t5\n"
-        + "            GROUP BY 'all') AS t8) AS $cor5,\n"
-        + "    LATERAL UNNEST (SELECT $cor5.X AS $f0\n"
-        + "        FROM (VALUES  (0)) AS t (ZERO)) "
+        + "            WHERE JOB <> 'CLERK'\n"
+        + "            ORDER BY SAL) AS t5\n"
+        + "        GROUP BY 'all') AS t8) AS $cor5,\n"
+        + "  LATERAL UNNEST (SELECT $cor5.X AS $f0\n"
+        + "    FROM (VALUES  (0)) AS t (ZERO)) "
         + "AS t11 (ENAME, JOB, DEPTNO, SAL) AS t110\n"
         + "ORDER BY $cor5.group";
     pig(script).assertRel(hasTree(plan))
@@ -510,12 +510,12 @@ public class PigRelOpTest extends PigRelTestBase {
         + "(7902,FORD,ANALYST,7566,1981-12-03,3000.00,null,20)\n";
     final String sql = ""
         + "SELECT *\n"
-        + "  FROM scott.EMP\n"
-        + "  WHERE DEPTNO = 10\n"
+        + "FROM scott.EMP\n"
+        + "WHERE DEPTNO = 10\n"
         + "UNION ALL\n"
-        + "  SELECT *\n"
-        + "  FROM scott.EMP\n"
-        + "  WHERE DEPTNO = 20";
+        + "SELECT *\n"
+        + "FROM scott.EMP\n"
+        + "WHERE DEPTNO = 20";
     pig(script).assertRel(hasTree(plan))
         .assertResult(is(result))
         .assertSql(is(sql));
@@ -548,11 +548,11 @@ public class PigRelOpTest extends PigRelTestBase {
         + "(40,OPERATIONS,null)\n";
     final String sql = ""
         + "SELECT *\n"
-        + "  FROM scott.DEPT\n"
+        + "FROM scott.DEPT\n"
         + "UNION ALL\n"
-        + "  SELECT DEPTNO, DNAME, "
+        + "SELECT DEPTNO, DNAME, "
         + "CAST(NULL AS VARCHAR CHARACTER SET ISO-8859-1) AS LOC\n"
-        + "  FROM scott.DEPT";
+        + "FROM scott.DEPT";
     pig(script).assertRel(hasTree(plan))
         .assertOptimizedRel(hasTree(optimizedPlan))
         .assertResult(is(result))
@@ -592,16 +592,16 @@ public class PigRelOpTest extends PigRelTestBase {
         + "SELECT EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, DEPTNO, "
         + "CAST(NULL AS VARCHAR CHARACTER SET ISO-8859-1) AS DNAME, "
         + "CAST(NULL AS VARCHAR CHARACTER SET ISO-8859-1) AS LOC\n"
-        + "  FROM scott.EMP\n"
-        + "  WHERE DEPTNO = 10\n"
+        + "FROM scott.EMP\n"
+        + "WHERE DEPTNO = 10\n"
         + "UNION ALL\n"
-        + "  SELECT CAST(NULL AS INTEGER) AS EMPNO, "
+        + "SELECT CAST(NULL AS INTEGER) AS EMPNO, "
         + "CAST(NULL AS VARCHAR CHARACTER SET ISO-8859-1) AS ENAME, "
         + "CAST(NULL AS VARCHAR CHARACTER SET ISO-8859-1) AS JOB, "
         + "CAST(NULL AS INTEGER) AS MGR, "
         + "CAST(NULL AS DATE) AS HIREDATE, CAST(NULL AS DECIMAL(19, 0)) AS SAL, "
         + "CAST(NULL AS DECIMAL(19, 0)) AS COMM, DEPTNO, DNAME, LOC\n"
-        + "  FROM scott.DEPT";
+        + "FROM scott.DEPT";
     pig(script).assertRel(hasTree(plan))
         .assertResult(is(result))
         .assertSql(is(sql));
@@ -720,8 +720,8 @@ public class PigRelOpTest extends PigRelTestBase {
         + "FROM scott.EMP\n"
         + "  INNER JOIN scott.DEPT ON EMP.DEPTNO = DEPT.DEPTNO\n"
         + "  INNER JOIN (SELECT *\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE LOC = 'CHICAGO') AS t ON EMP.DEPTNO = t.DEPTNO";
+        + "    FROM scott.DEPT\n"
+        + "    WHERE LOC = 'CHICAGO') AS t ON EMP.DEPTNO = t.DEPTNO";
     final String result = ""
         + "(7499,ALLEN,SALESMAN,7698,1981-02-20,1600.00,300.00,30,30,"
         + "SALES,CHICAGO,30,SALES,"
@@ -769,8 +769,8 @@ public class PigRelOpTest extends PigRelTestBase {
         + "  INNER JOIN scott.DEPT ON EMP.DEPTNO = DEPT.DEPTNO "
         + "AND EMP.ENAME = DEPT.DNAME\n"
         + "  INNER JOIN (SELECT *\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE LOC = 'CHICAGO') AS t ON EMP.DEPTNO = t.DEPTNO "
+        + "    FROM scott.DEPT\n"
+        + "    WHERE LOC = 'CHICAGO') AS t ON EMP.DEPTNO = t.DEPTNO "
         + "AND DEPT.DNAME = t.DNAME";
     pig(script2).assertRel(hasTree(plan2))
         .assertSql(is(sql2));
@@ -793,10 +793,10 @@ public class PigRelOpTest extends PigRelTestBase {
     final String sql = ""
         + "SELECT *\n"
         + "FROM (SELECT DEPTNO\n"
-        + "      FROM scott.DEPT) AS t,\n"
-        + "      (SELECT DEPTNO\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE DEPTNO <= 20) AS t1";
+        + "    FROM scott.DEPT) AS t,\n"
+        + "    (SELECT DEPTNO\n"
+        + "    FROM scott.DEPT\n"
+        + "    WHERE DEPTNO <= 20) AS t1";
     final String result = ""
         + "(10,10)\n"
         + "(10,20)\n"
@@ -850,13 +850,13 @@ public class PigRelOpTest extends PigRelTestBase {
     final String sql2 = ""
         + "SELECT *\n"
         + "FROM (SELECT DEPTNO\n"
-        + "      FROM scott.DEPT) AS t,\n"
-        + "      (SELECT DEPTNO\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE DEPTNO <= 20) AS t1,\n"
-        + "      (SELECT DEPTNO\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE DEPTNO > 20) AS t3";
+        + "    FROM scott.DEPT) AS t,\n"
+        + "    (SELECT DEPTNO\n"
+        + "    FROM scott.DEPT\n"
+        + "    WHERE DEPTNO <= 20) AS t1,\n"
+        + "    (SELECT DEPTNO\n"
+        + "    FROM scott.DEPT\n"
+        + "    WHERE DEPTNO > 20) AS t3";
     pig(script2).assertRel(hasTree(plan2))
         .assertResult(is(result2))
         .assertSql(is(sql2));
@@ -1591,19 +1591,21 @@ public class PigRelOpTest extends PigRelTestBase {
         + "AS DEPTNO, t4.A, t4.B, t7.C\n"
         + "FROM (SELECT CASE WHEN t0.$f0 IS NOT NULL THEN t0.$f0 ELSE t3.DEPTNO END "
         + "AS DEPTNO, t0.A, t3.B\n"
-        + "      FROM (SELECT DEPTNO + 10 AS $f0, COLLECT(ROW(DEPTNO, DNAME, LOC)) AS A\n"
-        + "            FROM scott.DEPT\n"
-        + "            GROUP BY DEPTNO + 10) AS t0\n"
-        + "        FULL JOIN (SELECT CAST(DEPTNO AS INTEGER) AS DEPTNO, COLLECT(ROW"
-        + "(DEPTNO, DNAME, LOC)) AS B\n"
-        + "            FROM scott.DEPT\n"
-        + "            WHERE DEPTNO <= 30\n"
-        + "            GROUP BY CAST(DEPTNO AS INTEGER)) AS t3 ON t0.$f0 = t3.DEPTNO) AS t4\n"
+        + "    FROM (SELECT DEPTNO + 10 AS $f0, "
+        + "COLLECT(ROW(DEPTNO, DNAME, LOC)) AS A\n"
+        + "        FROM scott.DEPT\n"
+        + "        GROUP BY DEPTNO + 10) AS t0\n"
+        + "      FULL JOIN (SELECT CAST(DEPTNO AS INTEGER) AS DEPTNO, "
+        + "COLLECT(ROW(DEPTNO, DNAME, LOC)) AS B\n"
+        + "        FROM scott.DEPT\n"
+        + "        WHERE DEPTNO <= 30\n"
+        + "        GROUP BY CAST(DEPTNO AS INTEGER)) AS t3 "
+        + "ON t0.$f0 = t3.DEPTNO) AS t4\n"
         + "  FULL JOIN (SELECT CAST(DEPTNO AS INTEGER) AS DEPTNO, COLLECT(ROW(DEPTNO, DNAME, "
         + "LOC)) AS C\n"
-        + "      FROM scott.DEPT\n"
-        + "      WHERE DEPTNO >= 20\n"
-        + "      GROUP BY CAST(DEPTNO AS INTEGER)) AS t7 ON t4.DEPTNO = t7.DEPTNO\n"
+        + "    FROM scott.DEPT\n"
+        + "    WHERE DEPTNO >= 20\n"
+        + "    GROUP BY CAST(DEPTNO AS INTEGER)) AS t7 ON t4.DEPTNO = t7.DEPTNO\n"
         + "ORDER BY CASE WHEN t4.DEPTNO IS NOT NULL THEN t4.DEPTNO ELSE t7.DEPTNO END";
     pig(script).assertRel(hasTree(plan))
         .assertResult(is(result))
diff --git a/server/src/main/java/org/apache/calcite/sql/ddl/SqlDdlNodes.java b/server/src/main/java/org/apache/calcite/sql/ddl/SqlDdlNodes.java
index 40fe4cc..d5ac495 100644
--- a/server/src/main/java/org/apache/calcite/sql/ddl/SqlDdlNodes.java
+++ b/server/src/main/java/org/apache/calcite/sql/ddl/SqlDdlNodes.java
@@ -30,7 +30,7 @@ import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlSelect;
-import org.apache.calcite.sql.dialect.CalciteSqlDialect;
+import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParserPos;
@@ -46,8 +46,6 @@ import org.apache.calcite.util.Util;
 
 import com.google.common.collect.ImmutableList;
 
-import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.util.List;
@@ -214,8 +212,7 @@ public class SqlDdlNodes {
       return query;
     }
     final SqlParserPos p = query.getParserPosition();
-    final SqlNodeList selectList =
-        new SqlNodeList(ImmutableList.<SqlNode>of(SqlIdentifier.star(p)), p);
+    final SqlNodeList selectList = SqlNodeList.SINGLETON_STAR;
     final SqlCall from =
         SqlStdOperatorTable.AS.createCall(p,
             ImmutableList.<SqlNode>builder()
@@ -238,16 +235,15 @@ public class SqlDdlNodes {
         .build();
     final Planner planner = Frameworks.getPlanner(config);
     try {
-      final StringWriter sw = new StringWriter();
-      final PrintWriter pw = new PrintWriter(sw);
-      final SqlPrettyWriter w =
-          new SqlPrettyWriter(CalciteSqlDialect.DEFAULT, false, pw);
-      pw.print("INSERT INTO ");
+      final StringBuilder buf = new StringBuilder();
+      final SqlWriterConfig writerConfig =
+          SqlPrettyWriter.config().withAlwaysUseParentheses(false);
+      final SqlPrettyWriter w = new SqlPrettyWriter(writerConfig, buf);
+      buf.append("INSERT INTO ");
       name.unparse(w, 0, 0);
-      pw.print(" ");
+      buf.append(' ');
       query.unparse(w, 0, 0);
-      pw.flush();
-      final String sql = sw.toString();
+      final String sql = buf.toString();
       final SqlNode query1 = planner.parse(sql);
       final SqlNode query2 = planner.validate(query1);
       final RelRoot r = planner.rel(query2);


[calcite] 02/07: Add gradle task 'aggregateJavadocIncludingTests' that builds javadoc for both main and test

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 4e7f92557a240de58cd950dce6ab478c1bc11604
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Wed Nov 20 14:10:48 2019 -0800

    Add gradle task 'aggregateJavadocIncludingTests' that builds javadoc for both main and test
    
    It will allow CI to spot javadoc errors, for example broken links, in test code.
---
 build.gradle.kts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/build.gradle.kts b/build.gradle.kts
index c67f112..a3dc229 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -143,6 +143,20 @@ val javadocAggregate by tasks.registering(Javadoc::class) {
     setDestinationDir(file("$buildDir/docs/javadocAggregate"))
 }
 
+/** Similar to {@link #javadocAggregate} but includes tests.
+ * CI uses this target to validate javadoc (e.g. checking for broken links). */
+val javadocAggregateIncludingTests by tasks.registering(Javadoc::class) {
+    description = "Generates aggregate javadoc for all the artifacts"
+
+    val sourceSets = allprojects
+        .mapNotNull { it.extensions.findByType<SourceSetContainer>() }
+        .flatMap { listOf(it.named("main"), it.named("test")) }
+
+    classpath = files(sourceSets.map { set -> set.map { it.output + it.compileClasspath } })
+    setSource(sourceSets.map { set -> set.map { it.allJava } })
+    setDestinationDir(file("$buildDir/docs/javadocAggregateIncludingTests"))
+}
+
 val adaptersForSqlline = listOf(
     ":babel", ":cassandra", ":druid", ":elasticsearch", ":file", ":geode", ":kafka", ":mongodb",
     ":pig", ":piglet", ":plus", ":spark", ":splunk"


[calcite] 03/07: Bump spark-core_2.10 from 2.2.0 to 2.2.2

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 8a49c9fda80c6d81721f0cf1044c83feed57f589
Author: dependabot[bot] <49...@users.noreply.github.com>
AuthorDate: Wed Nov 13 21:24:21 2019 +0000

    Bump spark-core_2.10 from 2.2.0 to 2.2.2
    
    Bumps spark-core_2.10 from 2.2.0 to 2.2.2.
    
    Signed-off-by: dependabot[bot] <su...@github.com>
---
 gradle.properties | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gradle.properties b/gradle.properties
index 2a7ccca..5b173dc 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -124,7 +124,7 @@ scott-data-hsqldb.version=0.1
 servlet.version=4.0.1
 sketches-core.version=0.9.0
 slf4j.version=1.7.25
-spark.version=2.2.0
+spark.version=2.2.2
 sqlline.version=1.9.0
 teradata.tpcds.version=1.2
 tpch.version=0.1


[calcite] 05/07: Javadoc warning

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 68eacfc57da7492377797e39ee3e2778be1b1aa4
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Tue Nov 12 10:50:51 2019 -0800

    Javadoc warning
    
    Also cosmetic changes
---
 .../java/org/apache/calcite/runtime/Resources.java |  4 +-
 .../calcite/sql/test/SqlOperatorBaseTest.java      |  9 ++-
 .../org/apache/calcite/test/RexProgramTest.java    | 11 ++-
 .../apache/calcite/test/SqlToRelConverterTest.java | 26 +++----
 .../test/enumerable/EnumerableCalcTest.java        |  3 +-
 .../calcite/test/fuzzer/RexProgramFuzzyTest.java   | 11 ++-
 .../adapter/elasticsearch/PredicateAnalyzer.java   |  1 +
 .../calcite/adapter/elasticsearch/MatchTest.java   | 80 ++++++++++++----------
 .../org/apache/calcite/adapter/tpch/TpchTest.java  |  5 +-
 9 files changed, 73 insertions(+), 77 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/runtime/Resources.java b/core/src/main/java/org/apache/calcite/runtime/Resources.java
index 8ec97ee..371b65e 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Resources.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Resources.java
@@ -99,7 +99,7 @@ public class Resources {
    *     ExInst&lt;IllegalArgumentException&gt; illegalBinaryString(String a0);
    * </blockquote>
    *
-   * will look up a resource "IllegalBinaryString" from the resource file
+   * <p>will look up a resource "IllegalBinaryString" from the resource file
    * "com/example/MyResource_en_US.properties", and substitute in the parameter
    * value {@code a0}.
    *
@@ -859,7 +859,7 @@ public class Resources {
    *
    * </blockquote>
    *
-   * Then when you call
+   * <p>Then when you call
    * {@link ResourceBundle#getBundle ResourceBundle.getBundle("foo.MyResource")},
    * it will find the class before the properties file, but still automatically
    * load the properties file based upon the name of the class.
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index edb5f3e..6559238 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -8928,14 +8928,13 @@ public abstract class SqlOperatorBaseTest {
   /** Test that calls all operators with all possible argument types, and for
    * each type, with a set of tricky values.
    *
-   * This is not really a unit test since there are no assertions;
+   * <p>This is not really a unit test since there are no assertions;
    * it either succeeds or fails in the preparation of the operator case
    * and not when actually testing (validating/executing) the call.
    *
-   * Nevertheless the log messages conceal many problems which potentially need
-   * to be fixed especially cases where the query passes from the validation stage
-   * and fails at runtime.
-   * */
+   * <p>Nevertheless the log messages conceal many problems which potentially
+   * need to be fixed especially cases where the query passes from the
+   * validation stage and fails at runtime. */
   @Disabled("Too slow and not really a unit test")
   @Tag("slow")
   @Test public void testArgumentBounds() {
diff --git a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
index 403067d..816cab6 100644
--- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
@@ -763,16 +763,13 @@ public class RexProgramTest extends RexProgramBuilderBase {
     checkSimplifyUnchanged(cast(cast(vVarchar(), tInt()), tVarchar()));
   }
 
-  @Disabled("CALCITE-3457: AssertionError in RexSimplify.validateStrongPolicy:843")
+  @Disabled("CALCITE-3457: AssertionError in RexSimplify.validateStrongPolicy")
   @Test public void reproducerFor3457() {
     // Identified with RexProgramFuzzyTest#testFuzzy, seed=4887662474363391810L
     checkSimplify(
-        eq(
-          unaryMinus(abstractCast(literal(1), tInt(true))),
-          unaryMinus(abstractCast(literal(1), tInt(true)))
-        ),
-        "I've no idea what I'm doing 🐕"
-    );
+        eq(unaryMinus(abstractCast(literal(1), tInt(true))),
+          unaryMinus(abstractCast(literal(1), tInt(true)))),
+        "true");
   }
 
   @Test public void testNoCommonReturnTypeFails() {
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index dcb80e9..c328085 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -133,9 +133,9 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).ok();
   }
 
-  /** Test case for:
+  /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-2468">[CALCITE-2468]
-   * struct type alias should not cause IOOBE.</a>.
+   * struct type alias should not cause IndexOutOfBoundsException</a>.
    */
   @Test public void testStructTypeAlias() {
     final String sql = "select t.r AS myRow\n"
@@ -167,11 +167,9 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).ok();
   }
 
-  /**
-   * Test case for
+  /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-245">[CALCITE-245]
-   * Off-by-one translation of ON clause of JOIN</a>.
-   */
+   * Off-by-one translation of ON clause of JOIN</a>. */
   @Test public void testConditionOffByOne() {
     // Bug causes the plan to contain
     //   LogicalJoin(condition=[=($9, $9)], joinType=[inner])
@@ -237,8 +235,7 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
 
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-801">[CALCITE-801]
-   * NullPointerException using USING on table alias with column
-   * aliases</a>. */
+   * NullPointerException using USING on table alias with column aliases</a>. */
   @Test public void testValuesUsing() {
     final String sql = "select d.deptno, min(e.empid) as empid\n"
         + "from (values (100, 'Bill', 1)) as e(empid, name, deptno)\n"
@@ -847,8 +844,7 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
 
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-439">[CALCITE-439]
-   * SqlValidatorUtil.uniquify() may not terminate under some
-   * conditions</a>. */
+   * SqlValidatorUtil.uniquify() may not terminate under some conditions</a>. */
   @Test public void testGroupAlias() {
     final String sql = "select \"$f2\", max(x), max(x + 1)\n"
         + "from (values (1, 2)) as t(\"$f2\", x)\n"
@@ -1992,7 +1988,7 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
 
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-3183">[CALCITE-3183]
-   * Trimming method for Filter rel uses wrong traitSet </a> */
+   * Trimming method for Filter rel uses wrong traitSet</a>. */
   @Test public void testFilterAndSortWithTrim() {
     // Create a customized test with RelCollation trait in the test cluster.
     Tester tester = new TesterImpl(getDiffRepos(),
@@ -2103,8 +2099,7 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
    *
    * <p>Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-551">[CALCITE-551]
-   * Sub-query inside aggregate function</a>.
-   */
+   * Sub-query inside aggregate function</a>. */
   @Test public void testAggCaseInSubQuery() {
     final String sql = "SELECT SUM(\n"
         + "  CASE WHEN deptno IN (SELECT deptno FROM dept) THEN 1 ELSE 0 END)\n"
@@ -3625,9 +3620,10 @@ public class SqlToRelConverterTest extends SqlToRelTestBase {
     sql(sql).ok();
   }
 
-  /** Test case for:
+  /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-3456">[CALCITE-3456]
-   * AssertionError throws when aggregation same digest in subquery in same scope</a>.
+   * AssertionError throws when aggregation same digest in sub-query in same
+   * scope</a>.
    */
   @Test public void testAggregateWithSameDigestInSubQueries() {
     final String sql = "select\n"
diff --git a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCalcTest.java b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCalcTest.java
index 1cf0cf5..631d352 100644
--- a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCalcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCalcTest.java
@@ -46,8 +46,7 @@ public class EnumerableCalcTest {
                     SqlStdOperatorTable.COALESCE,
                     builder.field("commission"),
                     builder.literal(0)))
-                .build()
-        )
+                .build())
         .planContains("inp4_ != null ? inp4_.intValue() : 0;")
         .returnsUnordered(
             "$f0=0",
diff --git a/core/src/test/java/org/apache/calcite/test/fuzzer/RexProgramFuzzyTest.java b/core/src/test/java/org/apache/calcite/test/fuzzer/RexProgramFuzzyTest.java
index afacada..2b56704 100644
--- a/core/src/test/java/org/apache/calcite/test/fuzzer/RexProgramFuzzyTest.java
+++ b/core/src/test/java/org/apache/calcite/test/fuzzer/RexProgramFuzzyTest.java
@@ -51,13 +51,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
- * Validates that {@link org.apache.calcite.rex.RexSimplify} is able to deal with
- * randomized {@link RexNode}.
- * Note: the default fuzzing time is 5 seconds to keep overall test duration reasonable.
- * The test starts from a random point every time, so the longer it runs the more errors it detects.
+ * Validates that {@link org.apache.calcite.rex.RexSimplify} is able to deal
+ * with a randomized {@link RexNode}.
  *
- * <p>Note: The test is not included to {@link org.apache.calcite.test.CalciteSuite} since it would
- * fail every build (there are lots of issues with {@link org.apache.calcite.rex.RexSimplify})
+ * <p>The default fuzzing time is 5 seconds to keep overall test duration
+ * reasonable. The test starts from a random point every time, so the longer it
+ * runs the more errors it detects.
  */
 public class RexProgramFuzzyTest extends RexProgramBuilderBase {
   protected static final Logger LOGGER =
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
index ade7b60..1679f03 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
@@ -52,6 +52,7 @@ import static org.apache.calcite.adapter.elasticsearch.QueryBuilders.regexpQuery
 import static org.apache.calcite.adapter.elasticsearch.QueryBuilders.termQuery;
 
 import static java.lang.String.format;
+
 /**
  * Query predicate analyzer. Uses visitor pattern to traverse existing expression
  * and convert it to {@link QueryBuilder}.
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
index 2e5efd3..ab3f0cb 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/MatchTest.java
@@ -16,7 +16,6 @@
  */
 package org.apache.calcite.adapter.elasticsearch;
 
-
 import org.apache.calcite.jdbc.CalciteConnection;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.type.RelDataType;
@@ -40,6 +39,7 @@ import org.apache.calcite.tools.RelRunner;
 import org.apache.calcite.util.NlsString;
 
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.LineProcessor;
 import com.google.common.io.Resources;
@@ -139,63 +139,68 @@ public class MatchTest {
     };
   }
 
-      /**
-   * Test the ElasticSearch match query. The match query is translated from CONTAINS query which
-   * is build using RelBuilder, RexBuilder because the normal sql query assumes CONTAINS query
-   * is for date/period range
+  /**
+   * Tests the ElasticSearch match query. The match query is translated from
+   * CONTAINS query which is build using RelBuilder, RexBuilder because the
+   * normal SQL query assumes CONTAINS query is for date/period range.
    *
-   * Equivalent SQL query: select * from zips where city contains 'waltham'
+   * <p>Equivalent SQL query:
    *
-   * ElasticSearch query for it:
-   * {"query":{"constant_score":{"filter":{"match":{"city":"waltham"}}}}}
+   * <blockquote>
+   * <code>select * from zips where city contains 'waltham'</code>
+   * </blockquote>
    *
-   * @throws Exception
+   * <p>ElasticSearch query for it:
+   *
+   * <blockquote><code>
+   * {"query":{"constant_score":{"filter":{"match":{"city":"waltham"}}}}}
+   * </code></blockquote>
    */
   @Test public void testMatchQuery() throws Exception {
 
     CalciteConnection con = (CalciteConnection) newConnectionFactory()
         .createConnection();
-    SchemaPlus postschema = con.getRootSchema().getSubSchema("elastic");
+    SchemaPlus postSchema = con.getRootSchema().getSubSchema("elastic");
 
     FrameworkConfig postConfig = Frameworks.newConfigBuilder()
-         .parserConfig(SqlParser.Config.DEFAULT)
-         .defaultSchema(postschema)
-         .build();
+        .parserConfig(SqlParser.Config.DEFAULT)
+        .defaultSchema(postSchema)
+        .build();
 
     final RelBuilder builder = RelBuilder.create(postConfig);
     builder.scan(ZIPS);
 
-    final RelDataTypeFactory relDataTypeFactory = new SqlTypeFactoryImpl(
-        RelDataTypeSystem.DEFAULT);
-    final RexBuilder rexbuilder = new RexBuilder(relDataTypeFactory);
+    final RelDataTypeFactory typeFactory =
+        new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+    final RexBuilder rexBuilder = new RexBuilder(typeFactory);
 
-    RexNode nameRexNode = rexbuilder.makeCall(SqlStdOperatorTable.ITEM,
-        rexbuilder.makeInputRef(relDataTypeFactory.createSqlType(SqlTypeName.ANY), 0),
-        rexbuilder.makeCharLiteral(
-          new NlsString("city", rexbuilder.getTypeFactory().getDefaultCharset().name(),
-          SqlCollation.COERCIBLE)));
+    RexNode nameRexNode = rexBuilder.makeCall(SqlStdOperatorTable.ITEM,
+        rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.ANY), 0),
+        rexBuilder.makeCharLiteral(
+            new NlsString("city", typeFactory.getDefaultCharset().name(),
+                SqlCollation.COERCIBLE)));
 
-    RelDataType mapType = relDataTypeFactory.createMapType(
-        relDataTypeFactory.createSqlType(SqlTypeName.VARCHAR),
-        relDataTypeFactory.createTypeWithNullability(
-          relDataTypeFactory.createSqlType(SqlTypeName.ANY), true));
+    RelDataType mapType = typeFactory.createMapType(
+        typeFactory.createSqlType(SqlTypeName.VARCHAR),
+        typeFactory.createTypeWithNullability(
+            typeFactory.createSqlType(SqlTypeName.ANY), true));
 
-    ArrayList<RexNode> namedList = new ArrayList<RexNode>(2);
-    namedList.add(rexbuilder.makeInputRef(mapType, 0));
-    namedList.add(nameRexNode);
+    List<RexNode> namedList =
+        ImmutableList.of(rexBuilder.makeInputRef(mapType, 0),
+            nameRexNode);
 
-    //Add fields in builder stack so it is accessible while filter preparation
+    // Add fields in builder stack so it is accessible while filter preparation
     builder.projectNamed(namedList, Arrays.asList("_MAP", "city"), true);
 
     RexNode filterRexNode = builder
-         .call(SqlStdOperatorTable.CONTAINS, builder.field("city"),
-         builder.literal("waltham"));
+        .call(SqlStdOperatorTable.CONTAINS, builder.field("city"),
+            builder.literal("waltham"));
     builder.filter(filterRexNode);
 
     String builderExpected = ""
-         + "LogicalFilter(condition=[CONTAINS($1, 'waltham')])\n"
-         + "  LogicalProject(_MAP=[$0], city=[ITEM($0, 'city')])\n"
-         + "    LogicalTableScan(table=[[elastic, " + ZIPS + "]])\n";
+        + "LogicalFilter(condition=[CONTAINS($1, 'waltham')])\n"
+        + "  LogicalProject(_MAP=[$0], city=[ITEM($0, 'city')])\n"
+        + "    LogicalTableScan(table=[[elastic, " + ZIPS + "]])\n";
 
     RelNode root = builder.build();
 
@@ -204,12 +209,13 @@ public class MatchTest {
 
       String s = CalciteAssert.toString(preparedStatement.executeQuery());
       final String result = ""
-          + "_MAP={id=02154, city=NORTH WALTHAM, loc=[-71.236497, 42.382492], pop=57871, state=MA}; city=NORTH WALTHAM\n";
+          + "_MAP={id=02154, city=NORTH WALTHAM, loc=[-71.236497, 42.382492], "
+          + "pop=57871, state=MA}; city=NORTH WALTHAM\n";
 
-      //Validate query prepared
+      // Validate query prepared
       assertThat(root, hasTree(builderExpected));
 
-      //Validate result returned from ES
+      // Validate result returned from ES
       assertThat(s, is(result));
     }
   }
diff --git a/plus/src/test/java/org/apache/calcite/adapter/tpch/TpchTest.java b/plus/src/test/java/org/apache/calcite/adapter/tpch/TpchTest.java
index 9a71d1f..933b764 100644
--- a/plus/src/test/java/org/apache/calcite/adapter/tpch/TpchTest.java
+++ b/plus/src/test/java/org/apache/calcite/adapter/tpch/TpchTest.java
@@ -929,11 +929,10 @@ public class TpchTest {
   }
 
   /** Runs with query #i.
-   *  @param i Ordinal of query, per the benchmark, 1-based
    *
-   */
+   * @param i Ordinal of query, per the benchmark, 1-based */
   private CalciteAssert.AssertQuery query(int i) {
     return with()
-      .query(QUERIES.get(i - 1).replaceAll("tpch\\.", "tpch_01."));
+        .query(QUERIES.get(i - 1).replaceAll("tpch\\.", "tpch_01."));
   }
 }


[calcite] 01/07: In DiffRepository, replace a synchronized Map with a Guava cache

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 8658804e0a1b5c2fe9c78fa8e40655db06531ac8
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Dec 16 15:57:53 2019 -0800

    In DiffRepository, replace a synchronized Map with a Guava cache
---
 .../org/apache/calcite/test/DiffRepository.java    | 62 +++++++++++++++-------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/core/src/test/java/org/apache/calcite/test/DiffRepository.java b/core/src/test/java/org/apache/calcite/test/DiffRepository.java
index fefa003..462c79f 100644
--- a/core/src/test/java/org/apache/calcite/test/DiffRepository.java
+++ b/core/src/test/java/org/apache/calcite/test/DiffRepository.java
@@ -22,6 +22,10 @@ import org.apache.calcite.util.Sources;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.XmlOutput;
 
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
 import org.junit.jupiter.api.Assertions;
 import org.opentest4j.AssertionFailedError;
 import org.w3c.dom.CDATASection;
@@ -39,10 +43,9 @@ import java.io.IOException;
 import java.io.Writer;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -161,8 +164,8 @@ public class DiffRepository {
    * the same class to share the same diff-repository: if the repository gets
    * loaded once per test case, then only one diff is recorded.
    */
-  private static final Map<Class, DiffRepository> MAP_CLASS_TO_REPOSITORY =
-      new HashMap<>();
+  private static final LoadingCache<Key, DiffRepository> REPOSITORY_CACHE =
+      CacheBuilder.newBuilder().build(CacheLoader.from(Key::toRepo));
 
   //~ Instance fields --------------------------------------------------------
 
@@ -755,22 +758,11 @@ public class DiffRepository {
    * @param filter    Filters each string returned by the repository
    * @return The diff repository shared between test cases in this class.
    */
-  public static synchronized DiffRepository lookup(
-      Class clazz,
+  public static DiffRepository lookup(Class clazz,
       DiffRepository baseRepository,
       Filter filter) {
-    DiffRepository diffRepository = MAP_CLASS_TO_REPOSITORY.get(clazz);
-    if (diffRepository == null) {
-      final URL refFile = findFile(clazz, ".xml");
-      final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
-      final File logFile =
-          new File(refFilePath.replace(".xml", "_actual.xml"));
-      assert !refFilePath.equals(logFile.getAbsolutePath());
-      diffRepository =
-          new DiffRepository(refFile, logFile, baseRepository, filter);
-      MAP_CLASS_TO_REPOSITORY.put(clazz, diffRepository);
-    }
-    return diffRepository;
+    final Key key = new Key(clazz, baseRepository, filter);
+    return REPOSITORY_CACHE.getUnchecked(key);
   }
 
   /**
@@ -794,4 +786,38 @@ public class DiffRepository {
         String text,
         String expanded);
   }
+
+  /** Cache key. */
+  private static class Key {
+    private final Class clazz;
+    private final DiffRepository baseRepository;
+    private final Filter filter;
+
+    Key(Class clazz, DiffRepository baseRepository, Filter filter) {
+      this.clazz = Objects.requireNonNull(clazz);
+      this.baseRepository = baseRepository;
+      this.filter = filter;
+    }
+
+    @Override public int hashCode() {
+      return Objects.hash(clazz, baseRepository, filter);
+    }
+
+    @Override public boolean equals(Object obj) {
+      return this == obj
+          || obj instanceof Key
+          && clazz.equals(((Key) obj).clazz)
+          && Objects.equals(baseRepository, ((Key) obj).baseRepository)
+          && Objects.equals(filter, ((Key) obj).filter);
+    }
+
+    DiffRepository toRepo() {
+      final URL refFile = findFile(clazz, ".xml");
+      final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
+      final String logFilePath = refFilePath.replace(".xml", "_actual.xml");
+      final File logFile = new File(logFilePath);
+      assert !refFilePath.equals(logFile.getAbsolutePath());
+      return new DiffRepository(refFile, logFile, baseRepository, filter);
+    }
+  }
 }


[calcite] 04/07: Contribute class Resources to ASF, and change its header

Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 61da868ea50fa9fd1584eaafd3fcb0c644e9c953
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Tue Dec 10 21:03:18 2019 -0800

    Contribute class Resources to ASF, and change its header
    
    Minor refactorings, such as converting anonymous classes to lambdas.
---
 .../java/org/apache/calcite/runtime/Resources.java | 83 +++++++---------------
 1 file changed, 27 insertions(+), 56 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/runtime/Resources.java b/core/src/main/java/org/apache/calcite/runtime/Resources.java
index f00a81e..8ec97ee 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Resources.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Resources.java
@@ -1,10 +1,10 @@
 /*
- * Licensed to Julian Hyde under one or more contributor license
- * agreements.  See the NOTICE file distributed with this work for
- * additional information regarding copyright ownership. Julian Hyde
- * licenses this file to you 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
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you 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
  *
  * http://www.apache.org/licenses/LICENSE-2.0
  *
@@ -23,6 +23,7 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.*;
+import java.security.PrivilegedAction;
 import java.text.*;
 import java.util.*;
 import java.util.concurrent.Callable;
@@ -30,18 +31,12 @@ import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Defining wrapper classes around resources that allow the compiler to check
- * whether the resources exist and have the argument types that your code
- * expects.
- *
- * <p>If this class belongs to a package other than
- * {@code net.hydromatic.resource}, it was probably generated by the Maven
- * plugin (groupId: "net.hydromatic", artifactId:
- * "hydromatic-resource-maven-plugin"). Code generation allows projects to use
- * this resource library without adding a runtime dependency on another JAR.
+ * whether the resources exist, and that uses of resources have the appropriate
+ * number and types of arguments to match the message.
  */
 public class Resources {
   private static final ThreadLocal<Locale> MAP_THREAD_TO_LOCALE =
-      new ThreadLocal<Locale>();
+      new ThreadLocal<>();
 
   private Resources() {}
 
@@ -140,8 +135,7 @@ public class Resources {
     return (T) Proxy.newProxyInstance(clazz.getClassLoader(),
         new Class[] {clazz},
         new InvocationHandler() {
-          final Map<String, Object> cache =
-              new ConcurrentHashMap<String, Object>();
+          final Map<String, Object> cache = new ConcurrentHashMap<>();
 
           public Object invoke(Object proxy, Method method, Object[] args)
               throws Throwable {
@@ -353,7 +347,7 @@ public class Resources {
           String raw = raw();
           MessageFormat format = new MessageFormat(raw);
           final Format[] formats = format.getFormatsByArgumentIndex();
-          final List<Class> types = new ArrayList<Class>();
+          final List<Class> types = new ArrayList<>();
           final Class<?>[] parameterTypes = method.getParameterTypes();
           for (int i = 0; i < formats.length; i++) {
             Format format1 = formats[i];
@@ -471,11 +465,8 @@ public class Resources {
           }
         }
         return ex;
-      } catch (InstantiationException e) {
-        throw new RuntimeException(e);
-      } catch (IllegalAccessException e) {
-        throw new RuntimeException(e);
-      } catch (NoSuchMethodException e) {
+      } catch (InstantiationException | IllegalAccessException
+          | NoSuchMethodException e) {
         throw new RuntimeException(e);
       } catch (InvocationTargetException e) {
         if (e.getCause() instanceof Error) {
@@ -528,35 +519,22 @@ public class Resources {
         if (ex == null) {
           cause = new NullPointerException();
         }
-      } catch (AssertionError e) {
-        cause = e;
-      } catch (RuntimeException e) {
-        cause = e;
-      } catch (Exception e) {
-        // This can never happen since exSupplier should be just a ex() call.
+      } catch (AssertionError | Exception e) {
         // catch(Exception) is required since Callable#call throws Exception.
-        // Just in case we get exception somehow, we will rethrow it as a part
-        // of AssertionError below.
+        // But in practice, e will always be AssertionError or RuntimeException,
+        // since exSupplier should be just a ex() call.
         cause = e;
       }
       if (cause != null) {
-        AssertionError assertionError = new AssertionError(
-            "error instantiating exception for resource '"
-                + method.getName() + "'");
-        assertionError.initCause(cause);
-        throw assertionError;
+        throw new AssertionError("error instantiating exception for resource '"
+            + method.getName() + "'", cause);
       }
     }
 
     @Override public void validate(EnumSet<Validation> validations) {
       super.validate(validations);
       if (validations.contains(Validation.CREATE_EXCEPTION)) {
-        validateException(
-            new Callable<Exception>() {
-              public Exception call() throws Exception {
-                return ex(new NullPointerException("test"));
-              }
-            });
+        validateException(() -> ex(new NullPointerException("test")));
       }
     }
   }
@@ -575,12 +553,7 @@ public class Resources {
     @Override public void validate(EnumSet<Validation> validations) {
       super.validate(validations);
       if (validations.contains(Validation.CREATE_EXCEPTION)) {
-        validateException(
-            new Callable<Exception>() {
-              public Exception call() throws Exception {
-                return ex();
-              }
-            });
+        validateException(this::ex);
       }
     }
   }
@@ -944,14 +917,12 @@ public class Resources {
     private static InputStream openPropertiesFile(Class clazz) {
       final ClassLoader loader = clazz.getClassLoader();
       final String resName = clazz.getName().replace('.', '/') + ".properties";
-      return (InputStream) java.security.AccessController.doPrivileged(
-          new java.security.PrivilegedAction() {
-            public Object run() {
-              if (loader != null) {
-                return loader.getResourceAsStream(resName);
-              } else {
-                return ClassLoader.getSystemResourceAsStream(resName);
-              }
+      return java.security.AccessController.doPrivileged(
+          (PrivilegedAction<InputStream>) () -> {
+            if (loader != null) {
+              return loader.getResourceAsStream(resName);
+            } else {
+              return ClassLoader.getSystemResourceAsStream(resName);
             }
           });
     }