wuchong commented on a change in pull request #12880:
URL: https://github.com/apache/flink/pull/12880#discussion_r466872581
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/ExecutionConfigOptions.java
##########
@@ -37,9 +39,24 @@
@PublicEvolving
public class ExecutionConfigOptions {
+ //
------------------------------------------------------------------------
+ // State Options
+ //
------------------------------------------------------------------------
+
+ @Documentation.TableOption(execMode = Documentation.ExecMode.STREAMING)
+ public static final ConfigOption<Duration> IDLE_STATE_RETENTION =
+ key("table.exec.state.ttl")
+ .durationType()
+ .defaultValue(Duration.ofMillis(0))
+ .withDescription("A time-to-live (TTL) can be assigned
to the keyed state of any type. " +
+ "If a TTL is configured and a state value has
expired, " +
+ "the stored value will be cleaned up on a best
effort basis. " +
+ "Default value is 0, which means that it will
never clean up state.");
Review comment:
```suggestion
.withDescription("Specifies a minimum time interval for
how long idle state " +
"(i.e. state which was not updated),
will be retained. State will never be " +
"cleared until it was idle for less
than the minimum time, and will be cleared " +
"at some time after it was idle.
Default is never clean-up the state.\n" +
"NOTE: Cleaning up state requires
additional overhead for bookkeeping.\n" +
"Default value is 0, which means that
it will never clean up state.");
```
##########
File path:
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableConfigTest.java
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.flink.table.api;
+
+import org.apache.flink.configuration.Configuration;
+
+import org.junit.Test;
+
+import java.time.Duration;
+import java.time.ZoneId;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Tests for {@link TableConfig}.
+ */
+public class TableConfigTest {
Review comment:
We don't need to refactor it into `TestSpec`. There is limited methods
to test. We can have a test for each option.
The TestSpec is not easy to understand and can't test some special methods,
e.g. `setIdleStateRetentionTime(Time minTime, Time maxTime)`
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java
##########
@@ -51,4 +52,24 @@ private TableConfigOptions() {}
.withDescription("The SQL dialect defines how to parse
a SQL query. " +
"A different SQL dialect may support
different SQL grammar. " +
"Currently supported dialects are:
default and hive");
+
+ @Documentation.TableOption(execMode =
Documentation.ExecMode.BATCH_STREAMING)
+ public static final ConfigOption<String> LOCAL_TIME_ZONE =
key("table.local-time-zone")
+ .stringType()
+ // special value to decide whether to use
ZoneId.systemDefault() in TableConfig.getLocalTimeZone()
+ .defaultValue("default")
+ .withDescription("The local time zone defines current
session time zone id. It is used when converting to/from " +
+ "<code>TIMESTAMP_WITH_LOCAL_TIME_ZONE</code>.
Internally, timestamps with local time zone are always represented in the UTC
time zone. " +
+ "However, when converting to data types that
don't include a time zone (e.g. TIMESTAMP, TIME, or simply STRING), " +
+ "the session time zone is used during
conversion. The input of option is either an abbreviation such as \"PST\", a
full name " +
+ "such as \"America/Los_Angeles\", or a custom
timezone_id such as \"GMT-8:00\".");
Review comment:
```suggestion
.withDescription("The local time zone defines current
session time zone id. It is used when converting to/from " +
"<code>TIMESTAMP WITH LOCAL TIME ZONE</code>.
Internally, timestamps with local time zone are always represented in the UTC
time zone. " +
"However, when converting to data types that
don't include a time zone (e.g. TIMESTAMP, TIME, or simply STRING), " +
"the session time zone is used during
conversion. The input of option is either an abbreviation such as \"PST\", a
full name " +
"such as \"America/Los_Angeles\", or a custom
timezone id such as \"GMT-8:00\".");
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
