[GitHub] flink issue #6263: [FLINK-9743][Client] Use correct zip/jar path separator
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6263 ok, thank you for review about `public TemporaryFolder temporaryFolder = new TemporaryFolder();` could be final - agree. However while searching for usages I realized that in most cases it is not final but not changed. Am I right that in spite of existing code at least for newly created test classes it is better to use temporaryFolder as final? ---
[GitHub] flink issue #6250: [FLINK-8864][Table API & SQL] added command history for S...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6250 cc @twalthr as you are the issue reporter could you please have a look here? ---
[GitHub] flink issue #6161: [hotfix] [docs][FLINK-9581] Typo: extra spaces removed to...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6161 @fhueske thank you for review @tillrohrmann if you took #6258 and @fhueske reviewed this may be it also makes sense to take into 1.5.1? ---
[GitHub] flink pull request #6263: [FLINK-9743][Client] Use correct zip/jar path sepa...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6263 [FLINK-9743][Client] Use correct zip/jar path separator ## What is the purpose of the change *This PR resolves libraries extraction issue from jars* ## Brief change log - *Always in case of zip/jar use '/' path separator* - *Test with generated jar emulating the real case* ## Verifying this change - Added test generates fake jar with a structure test.jar |- lib |--|- internalTest.jar and then calls for `PackagedProgram#extractContainedLibraries` to check if it extracts internalTest.jar correctly ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_9743 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6263.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6263 commit 649b9f1a0893aea32bd2dc3cfa1702bfb77ab29f Author: snuyanzin Date: 2018-07-05T08:58:33Z [FLINK-9743] use correct zip path separator, PackagedProgramTest#testExtractContainedLibraries to check PackagedProgram#extractContainedLibraries ---
[GitHub] flink pull request #6258: [hotfix][docs][FLINK-9757] fix typos
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6258#discussion_r200209274
--- Diff: docs/dev/table/sql.md ---
@@ -2656,7 +2656,7 @@ Although not every SQL feature is implemented yet,
some string combinations are
{% highlight sql %}
-A, ABS, ABSOLUTE, ACTION, ADA, ADD, ADMIN, AFTER, ALL, ALLOCATE, ALLOW,
ALTER, ALWAYS, AND, ANY, ARE, ARRAY, AS, ASC, ASENSITIVE, ASSERTION,
ASSIGNMENT, ASYMMETRIC, AT, ATOMIC, ATTRIBUTE, ATTRIBUTES, AUTHORIZATION, AVG,
BEFORE, BEGIN, BERNOULLI, BETWEEN, BIGINT, BINARY, BIT, BLOB, BOOLEAN, BOTH,
BREADTH, BY, C, CALL, CALLED, CARDINALITY, CASCADE, CASCADED, CASE, CAST,
CATALOG, CATALOG_NAME, CEIL, CEILING, CENTURY, CHAIN, CHAR, CHARACTER,
CHARACTERISTICTS, CHARACTERS, CHARACTER_LENGTH, CHARACTER_SET_CATALOG,
CHARACTER_SET_NAME, CHARACTER_SET_SCHEMA, CHAR_LENGTH, CHECK, CLASS_ORIGIN,
CLOB, CLOSE, COALESCE, COBOL, COLLATE, COLLATION, COLLATION_CATALOG,
COLLATION_NAME, COLLATION_SCHEMA, COLLECT, COLUMN, COLUMN_NAME,
COMMAND_FUNCTION, COMMAND_FUNCTION_CODE, COMMIT, COMMITTED, CONDITION,
CONDITION_NUMBER, CONNECT, CONNECTION, CONNECTION_NAME, CONSTRAINT,
CONSTRAINTS, CONSTRAINT_CATALOG, CONSTRAINT_NAME, CONSTRAINT_SCHEMA,
CONSTRUCTOR, CONTAINS, CONTINUE, CONVERT, CORR, CORRESPONDING,
COUNT, COVAR_POP, COVAR_SAMP, CREATE, CROSS, CUBE, CUME_DIST, CURRENT,
CURRENT_CATALOG, CURRENT_DATE, CURRENT_DEFAULT_TRANSFORM_GROUP, CURRENT_PATH,
CURRENT_ROLE, CURRENT_SCHEMA, CURRENT_TIME, CURRENT_TIMESTAMP,
CURRENT_TRANSFORM_GROUP_FOR_TYPE, CURRENT_USER, CURSOR, CURSOR_NAME, CYCLE,
DATA, DATABASE, DATE, DATETIME_INTERVAL_CODE, DATETIME_INTERVAL_PRECISION, DAY,
DEALLOCATE, DEC, DECADE, DECIMAL, DECLARE, DEFAULT, DEFAULTS, DEFERRABLE,
DEFERRED, DEFINED, DEFINER, DEGREE, DELETE, DENSE_RANK, DEPTH, DEREF, DERIVED,
DESC, DESCRIBE, DESCRIPTION, DESCRIPTOR, DETERMINISTIC, DIAGNOSTICS, DISALLOW,
DISCONNECT, DISPATCH, DISTINCT, DOMAIN, DOUBLE, DOW, DOY, DROP, DYNAMIC,
DYNAMIC_FUNCTION, DYNAMIC_FUNCTION_CODE, EACH, ELEMENT, ELSE, END, END-EXEC,
EPOCH, EQUALS, ESCAPE, EVERY, EXCEPT, EXCEPTION, EXCLUDE, EXCLUDING, EXEC,
EXECUTE, EXISTS, EXP, EXPLAIN, EXTEND, EXTERNAL, EXTRACT, FALSE, FETCH, FILTER,
FINAL, FIRST, FIRST_VALUE, FLOAT, FLOOR, FOLLOWING, FOR, FOREIGN, FORTRAN,
FOUND, FRAC_SECON
D, FREE, FROM, FULL, FUNCTION, FUSION, G, GENERAL, GENERATED, GET, GLOBAL, GO,
GOTO, GRANT, GRANTED, GROUP, GROUPING, HAVING, HIERARCHY, HOLD, HOUR, IDENTITY,
IMMEDIATE, IMPLEMENTATION, IMPORT, IN, INCLUDING, INCREMENT, INDICATOR,
INITIALLY, INNER, INOUT, INPUT, INSENSITIVE, INSERT, INSTANCE, INSTANTIABLE,
INT, INTEGER, INTERSECT, INTERSECTION, INTERVAL, INTO, INVOKER, IS, ISOLATION,
JAVA, JOIN, K, KEY, KEY_MEMBER, KEY_TYPE, LABEL, LANGUAGE, LARGE, LAST,
LAST_VALUE, LATERAL, LEADING, LEFT, LENGTH, LEVEL, LIBRARY, LIKE, LIMIT, LN,
LOCAL, LOCALTIME, LOCALTIMESTAMP, LOCATOR, LOWER, M, MAP, MATCH, MATCHED, MAX,
MAXVALUE, MEMBER, MERGE, MESSAGE_LENGTH, MESSAGE_OCTET_LENGTH, MESSAGE_TEXT,
METHOD, MICROSECOND, MILLENNIUM, MIN, MINUTE, MINVALUE, MOD, MODIFIES, MODULE,
MONTH, MORE, MULTISET, MUMPS, NAME, NAMES, NATIONAL, NATURAL, NCHAR, NCLOB,
NESTING, NEW, NEXT, NO, NONE, NORMALIZE, NORMALIZED, NOT, NULL, NULLABLE,
NULLIF, NULLS, NUMBER, NUMERIC, OBJECT, OCTETS, OCTET_LENGTH, OF, OFFSET, OL
D, ON, ONLY, OPEN, OPTION, OPTIONS, OR, ORDER, ORDERING, ORDINALITY, OTHERS,
OUT, OUTER, OUTPUT, OVER, OVERLAPS, OVERLAY, OVERRIDING, PAD, PARAMETER,
PARAMETER_MODE, PARAMETER_NAME, PARAMETER_ORDINAL_POSITION,
PARAMETER_SPECIFIC_CATALOG, PARAMETER_SPECIFIC_NAME, PARAMETER_SPECIFIC_SCHEMA,
PARTIAL, PARTITION, PASCAL, PASSTHROUGH, PATH, PERCENTILE_CONT,
PERCENTILE_DISC, PERCENT_RANK, PLACING, PLAN, PLI, POSITION, POWER, PRECEDING,
PRECISION, PREPARE, PRESERVE, PRIMARY, PRIOR, PRIVILEGES, PROCEDURE, PUBLIC,
QUARTER, RANGE, RANK, READ, READS, REAL, RECURSIVE, REF, REFERENCES,
REFERENCING, REGR_AVGX, REGR_AVGY, REGR_COUNT, REGR_INTERCEPT, REGR_R2,
REGR_SLOPE, REGR_SXX, REGR_SXY, REGR_SYY, RELATIVE, RELEASE, REPEATABLE, RESET,
RESTART, RESTRICT, RESULT, RETURN, RETURNED_CARDINALITY, RETURNED_LENGTH,
RETURNED_OCTET_LENGTH, RETURNED_SQLSTATE, RETURNS, REVOKE, RIGHT, ROLE,
ROLLBACK, ROLLUP, ROUTINE, ROUTINE_CATALOG, ROUTINE_NAME, ROUTINE_SCHEMA, ROW,
ROWS, ROW_COUNT, ROW_NUMBER, SAVEPOINT, S
CALE, SCHEMA, SCHEMA_NAME, SCOPE, SCOPE_CATALOGS, SCOPE_NAME, SCOPE_SCHEMA,
SCROLL, SEARCH, SECOND, SECTION, SECURITY, SELECT, SELF, SENSITIVE, SEQUENCE,
SERIALIZABLE, SERVER, SERVER_NAME, SESSION, SESSION_USER, SET, SETS, SIMILAR,
SIMPLE, SIZE, SMALLINT, SOME, SOURCE, SPACE, SPECIFIC, SPECIFICTYPE,
SPECIFIC_NAME, SQL, SQLEXCEPTION, SQLSTATE, SQLWARNING, SQL_TSI_DAY,
SQL_TSI_FRAC_SECOND, SQL_TSI_HOUR, SQL_TSI_MICROSECOND, SQL_TSI_MINUTE,
SQL_TSI_MONTH, SQL_TSI_QUARTER, SQL_TSI_SECOND, SQL_TSI_WEEK, SQL_TSI_YEAR,
SQRT, START, STATE, STATEMENT, STATIC, STDDEV_POP, STDDEV_SAMP, STREAM,
STRUCTURE, STYLE, SUBCLASS_ORIGIN
[GitHub] flink pull request #6258: [hotfix][docs][FLINK-9757] fix typos
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6258 [hotfix][docs][FLINK-9757] fix typos ## What is the purpose of the change *This PR corrects typos found while hunspell run* ## Brief change log typos from [FLINK-9757] description ## Documentation - Does this pull request introduce a new feature? ( no) - If yes, how is the feature documented? (not applicable ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink TYPOS Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6258.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6258 commit 87a3c33166820087b34e7afd1da0f9b608bfe9dc Author: snuyanzin Date: 2018-07-04T16:24:50Z ]FLINK-9757] Fix typos ---
[GitHub] flink pull request #6250: [FLINK-8864] added command history
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6250 [FLINK-8864] added command history ## What is the purpose of the change *This PR adds history for sql queries/sqlclient commands* ## Brief change log - *Added history file for commands* ## Verifying this change - *Manually verification* 1. run sql-sclient, perform whatever queries, close close client 2. run sql-client again, check history (up, down, forward and backward searches via ctrl+s, ctrl+r) ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_8864 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6250.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6250 commit 08ee97e1c4b0274b8508e6cfb6f47db1e33212eb Author: snuyanzin Date: 2018-07-04T00:30:14Z [FLINK-8864] added command history ---
[GitHub] flink pull request #6249: [hotfix] [docs] [FLINK-9734] Fix 'deleimiter' typo
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6249 [hotfix] [docs] [FLINK-9734] Fix 'deleimiter' typo ## What is the purpose of the change *This PR fixes typo in deleimeter word at https://ci.apache.org/projects/flink/flink-docs-master/dev/table/sqlClient.html#csv-format* ## Brief change log - *type fix at docs/dev/table/sqlClient.md* You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FIELD_DELIMETER_TYPO Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6249.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6249 commit c538c8585ee66585bdf61cfa326668f82bb0e532 Author: snuyanzin Date: 2018-07-03T17:40:56Z [FLINK-9734] Fix 'deleimiter' typo ---
[GitHub] flink pull request #6223: [FLINK-9688] ATAN2 sql function support
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6223#discussion_r199866207
--- Diff: docs/dev/table/tableApi.md ---
@@ -2184,6 +2184,17 @@ NUMERIC.atan()
+
+
+{% highlight java %}
+NUMERIC.atan2(NUMERIC)
--- End diff --
Agree I changed the syntax to `atan2(Numeric, Numeric)`
---
[GitHub] flink pull request #6246: [hotfix][FLINK-9729][docs][Table API & SQL] Remove...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6246 [hotfix][FLINK-9729][docs][Table API & SQL] Remove duplicate row for %W ## What is the purpose of the change *This PR removes duplicate line for `%W Weekday name (Sunday .. Saturday)` from documentation* ## Brief change log *(for example:)* - *lines removed from docs/dev/table/sql.md* ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink WEEKDAY_TYPO Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6246.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6246 commit 5c42513726b7a7441f2540bf2112144f9feee246 Author: snuyanzin Date: 2018-07-03T13:53:22Z Remove duplicate row for %W Weekday name (Sunday .. Saturday) ---
[GitHub] flink issue #6223: [FLINK-9688] ATAN2 sql function support
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6223 @fhueske @hequn8128 @yanghua thank you very much for your review and comments I did almost all the corrections except one related to atan2 description So I have a question: as @hequn8128 wrote > I checked the [wiki](https://en.wikipedia.org/wiki/Atan2) about atan2, it said: The atan2 function calculates one unique arc tangent value from two variables y and x. So, would it be better of two variables? atan2(y,x) can be the arc tangent of (x,y) or (nx, ny). At the same time I checked [Calcite's definition](https://calcite.apache.org/docs/reference.html) of it: _ATAN2(numeric, numeric) | Returns the arc tangent of the numeric coordinates._ What do you think what is more suitable? ---
[GitHub] flink issue #6233: [FLINK-9696] Deep toString for array/map sql types
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6233 @hequn8128 thank you for your idea about Row. I used it in tests. Also I checked usages of `org.apache.flink.table.client.cli.CliStrings#NULL_COLUMN` it was added only recently and used only in CliUtils before. For that reason I think it could be set to "null" what I did in PR. @twalthr @hequn8128 please let me know if you have any objections ---
[GitHub] flink pull request #6233: [FLINK-9696] Deep toString for array/map sql types
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6233#discussion_r199559926
--- Diff:
flink-libraries/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliUtilsTest.java
---
@@ -0,0 +1,108 @@
+/*
+ * 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.client.cli;
+
+import org.apache.flink.api.java.tuple.Tuple3;
+import org.apache.flink.types.Row;
+
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests for {@link CliUtils}.
+ */
+public class CliUtilsTest {
+
+ @Test
+ public void testRowToString() throws IOException {
+ Row result = new Row(10);
+ result.setField(0, null);
+ result.setField(1, "String");
+ result.setField(2, 'c');
+ result.setField(3, false);
+ result.setField(4, 12345.67f);
+ result.setField(5, 12345.67d);
+ result.setField(6, 12345L);
+ result.setField(7, java.sql.Date.valueOf("2018-11-12"));
+ result.setField(8, new int[]{1, 2});
+ result.setField(9, new Tuple3<>(1, "123", null));
+ assertEquals(Arrays.toString(CliUtils.rowToString(result)),
--- End diff --
Thank you for the catch.
Corrected
---
[GitHub] flink pull request #6223: [FLINK-9688] ATAN2 sql function support
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6223#discussion_r199543936
--- Diff: docs/dev/table/sql.md ---
@@ -1510,6 +1510,17 @@ ATAN(numeric)
+
+
+{% highlight text %}
+ATAN2(numeric, numeric)
+{% endhighlight %}
+
+
+Calculates the arc tangent of a given coordinate.
--- End diff --
Hello @hequn8128! The idea about coordinates comes from Math.atan2 +
Calcite's definition here [1] ATAN2(numeric, numeric) | Returns the arc tangent
of the numeric coordinates.
Does it make sense to take the same wording as Calcite has?
[1] https://calcite.apache.org/docs/reference.html
---
[GitHub] flink pull request #6223: [FLINK-9688] ATAN2 sql function support
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6223#discussion_r199366673
--- Diff:
flink-libraries/flink-table/src/test/scala/org/apache/flink/table/expressions/ScalarFunctionsTest.scala
---
@@ -,6 +,53 @@ class ScalarFunctionsTest extends
ScalarTypesTestBase {
math.atan(-0.123123132132132).toString)
}
+ @Test
+ def testAtan2(): Unit = {
+testAllApis(
+ 'f25.atan2('f26),
+ "f25.atan2(f26)",
+ "ATAN2(f25, f26)",
+ math.atan2(0.42.toByte, 0.toByte).toString)
+
+
--- End diff --
Thank you for that catch.
blank line removed
---
[GitHub] flink issue #6233: [FLINK-9696] Deep toString for array/map sql types
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6233 Hello @hequn8128! Thank you for your review and comments. About PR template - I did changes based on proposed #5811. Please let me know if it is acceptable or not. About `rowToString` agree. I think it makes sense and I added such tests. However I faced with some strange behavior (I do not know if it is bug or whatever else). Commented on the code about that. ---
[GitHub] flink pull request #6233: [FLINK-9696] Deep toString for array/map sql types
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6233#discussion_r199364538
--- Diff:
flink-libraries/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliUtilsTest.java
---
@@ -0,0 +1,108 @@
+/*
+ * 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.client.cli;
+
+import org.apache.flink.api.java.tuple.Tuple3;
+import org.apache.flink.types.Row;
+
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests for {@link CliUtils}.
+ */
+public class CliUtilsTest {
+
+ @Test
+ public void testRowToString() throws IOException {
+ Row result = new Row(10);
+ result.setField(0, null);
+ result.setField(1, "String");
+ result.setField(2, 'c');
+ result.setField(3, false);
+ result.setField(4, 12345.67f);
+ result.setField(5, 12345.67d);
+ result.setField(6, 12345L);
+ result.setField(7, java.sql.Date.valueOf("2018-11-12"));
+ result.setField(8, new int[]{1, 2});
+ result.setField(9, new Tuple3<>(1, "123", null));
--- End diff --
Is it a real case to have tuple here for SqlClient? API allows to do that
but not sure about real cases.
---
[GitHub] flink pull request #6233: [FLINK-9696] Deep toString for array/map sql types
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6233#discussion_r199364581
--- Diff:
flink-libraries/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliUtilsTest.java
---
@@ -0,0 +1,108 @@
+/*
+ * 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.client.cli;
+
+import org.apache.flink.api.java.tuple.Tuple3;
+import org.apache.flink.types.Row;
+
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests for {@link CliUtils}.
+ */
+public class CliUtilsTest {
+
+ @Test
+ public void testRowToString() throws IOException {
+ Row result = new Row(10);
+ result.setField(0, null);
+ result.setField(1, "String");
+ result.setField(2, 'c');
+ result.setField(3, false);
+ result.setField(4, 12345.67f);
+ result.setField(5, 12345.67d);
+ result.setField(6, 12345L);
+ result.setField(7, java.sql.Date.valueOf("2018-11-12"));
+ result.setField(8, new int[]{1, 2});
+ result.setField(9, new Tuple3<>(1, "123", null));
+ assertEquals(Arrays.toString(CliUtils.rowToString(result)),
+ "[(NULL), String, c, false, 12345.67, 12345.67, 12345,
2018-11-12, " +
+ "[1, 2], (1,123,null)]");
--- End diff --
If having tuple here is ok then the next strange thing is null handling
inside tuples (it is printed in lowercase and without brackets). So there are
at least 2 different types of null handling: inside tuples and all others.
---
[GitHub] flink pull request #6233: [FLINK-9696] Deep toString for array/map sql types
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6233 [FLINK-9696] Deep toString for array/map sql types *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change *(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)* ## Brief change log *(for example:)* - *Make it possible to have array/map types human readable* ## Verifying this change *run queries from from the description* ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_9696 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6233.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6233 commit 97b648cc87a494c95f4a5146891285e85db3ae1b Author: snuyanzin Date: 2018-06-30T14:42:57Z [FLINK-9696] Deep toString for array/map sql types ---
[GitHub] flink issue #6226: [FLINK-8650] Tests for WINDOW clause and documentation up...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6226 @fhueske thank you for your review I have left comments and done changes. Please let me know if it is required some more efforts ---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6226#discussion_r199223615
--- Diff: docs/dev/table/sql.md ---
@@ -176,9 +181,20 @@ groupItem:
| ROLLUP '(' expression [, expression ]* ')'
| GROUPING SETS '(' groupItem [, groupItem ]* ')'
-insert:
- INSERT INTO tableReference
- query
+windowRef:
+ windowName
+ | windowSpec
+
+windowSpec:
+ [ windowName ]
+ '('
+ [ ORDER BY orderItem [, orderItem ]* ]
+ [ PARTITION BY expression [, expression ]* ]
+ [
+ RANGE numericOrIntervalExpression { PRECEDING | FOLLOWING }
--- End diff --
Yes agree I've removed it
---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6226#discussion_r199223349
--- Diff:
flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/stream/sql/OverWindowTest.scala
---
@@ -44,7 +44,26 @@ class OverWindowTest extends TableTestBase {
"sum(DISTINCT c) OVER (PARTITION BY b ORDER BY proctime ROWS BETWEEN
2 preceding AND " +
"CURRENT ROW) as sum2 " +
"from MyTable"
-
+val sql2 = "SELECT " +
--- End diff --
Agree. I've reduced number of queries per test case (one without `WINDOW `
and one with). Please let me know if it looks good or not
---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6226#discussion_r199222853
--- Diff: docs/dev/table/sql.md ---
@@ -176,9 +181,20 @@ groupItem:
| ROLLUP '(' expression [, expression ]* ')'
| GROUPING SETS '(' groupItem [, groupItem ]* ')'
-insert:
- INSERT INTO tableReference
- query
+windowRef:
+ windowName
+ | windowSpec
+
+windowSpec:
--- End diff --
yes sure, done
---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6226#discussion_r198928207
--- Diff: docs/dev/table/sql.md ---
@@ -115,6 +115,10 @@ The following BNF-grammar describes the superset of
supported SQL features in ba
{% highlight sql %}
+insert:
+ INSERT INTO tableReference
--- End diff --
Move up to be sync with the similar Calcite's doc
---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6226#discussion_r198928117
--- Diff: docs/dev/table/sql.md ---
@@ -139,7 +143,8 @@ select:
[ WHERE booleanExpression ]
[ GROUP BY { groupItem [, groupItem ]* } ]
[ HAVING booleanExpression ]
-
+ [ WINDOW windowName AS windowSpec [, windowName AS windowSpec ]* ]
--- End diff --
Done in a way Calcite does it
---
[GitHub] flink pull request #6226: [FLINK-8650] Tests for WINDOW clause and documenta...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6226 [FLINK-8650] Tests for WINDOW clause and documentation update *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change test and documentation coverage of WINDOW clause ## Brief change log - *Test that the same queries but with different specification of windows have the same plan* - *Mentioning in doc WINDOW syntax* ## Verifying this change This change added tests and can be verified as follows: via running of org.apache.flink.table.api.stream.sql.OverWindowTest ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_8560 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6226.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6226 commit 85451a2f9e6d1cd5d8b98fa2bba5c92326e817ce Author: snuyanzin Date: 2018-06-28T16:19:25Z [FLINK-8650] Tests for WINDOW clause and documentation update ---
[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/5961 commits squashed into one ---
[GitHub] flink pull request #6161: [FLINK-9581] Remove extra spaces to make COLLECT l...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6161 [FLINK-9581] Remove extra spaces to make COLLECT left aligned ## Brief change log extra spaces removed ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): ( no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: ( no ) - The runtime per-record code paths (performance sensitive): ( no ) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: ( no ) - The S3 file system connector: ( no ) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink COLLECT_DOC_TYPO Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6161.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6161 commit 009afd05bc1316f35dd9841ac6367aab101d2970 Author: snuyanzin Date: 2018-06-13T17:30:26Z Remove trailing space to make COLLECT left aligned ---
[GitHub] flink pull request #6099: [FLINK-9473][Table API & SQL] Added new methods in...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6099 [FLINK-9473][Table API & SQL] Added new methods into ExternalCatalogSchema based on interface Schema changes in Calcite ## What is the purpose of the change Make Flink compilable after Calcite version bump to 1.17.0 ## Brief change log - *new abstract methods dummy implementation* - *bump calcite version to 1.17.0-SNAPSHOT* ## Verifying this change to verify need to check if it or compilable or not e.g. *mvn clean package -DskipTests* This change added tests and can be verified as follows: ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no ) - The runtime per-record code paths (performance sensitive): (no ) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no ) - The S3 file system connector: (no ) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK_9473 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6099.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6099 commit 61fda81fa46a7594c9a54450b525a5c321866324 Author: snuyanzin Date: 2018-05-30T06:05:24Z Make Flink compilable after bump calcite version to 1.17 ---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTRACT
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 > Just because DOW is in the reserved keywords section doesn't mean that we support it. Definitely agree however my point was that the same situation is for DOY, CENTURY and others... Now I added information about date/time functions synonyms which currently work in flink. I was surprised but they work without any efforts => I just added more tests related to them and documentation. @twalthr could you please have a look? ---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTRACT
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 @twalthr thank you for your comment but could you please clarify this > Can you add some documentation to sqlApi.md such that users know about the semantics? 1. Am I right that you mean `docs/dev/table/sql.md`? 2. Currently I see the general explanation of extract and reserved words where DOW already specified. From this point of view I do not see what could be updated right now. At the same time I have a proposal to go to the similar way as Calcite does. [Here](https://calcite.apache.org/docs/reference.html) there is a link to their functions including date/time. Among extract they also have synonyms e.g. >MONTH(date) | Equivalent to EXTRACT(MONTH FROM date). Returns an integer between 1 and 12. >WEEK(date) | Equivalent to EXTRACT(WEEK FROM date). Returns an integer between 1 and 53. >DAYOFYEAR(date) | Equivalent to EXTRACT(DOY FROM date). Returns an integer between 1 and 366. and etc. So I suggest to introduce the same synonyms in flink (just via usage of existing in Calcite) and organize documentation for them in a similar way ---
[GitHub] flink pull request #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTR...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6007#discussion_r190299515
--- Diff:
flink-libraries/flink-table/src/test/scala/org/apache/flink/table/expressions/ScalarFunctionsTest.scala
---
@@ -1473,6 +1473,14 @@ class ScalarFunctionsTest extends
ScalarTypesTestBase {
"EXTRACT(DOY FROM f16)",
"315")
+testSqlApi(
+ "EXTRACT(DOW FROM f18)",
+ "1")
+
+testSqlApi(
+ "EXTRACT(DOW FROM f16)",
--- End diff --
Yes sure, just have done it for DOW and DOY as well
---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTRACT
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 @twalthr thank you for your comment. I took a similar ticket in Calcite/Avatica from which the current depends on and I pointed this discrepancy in the [comment ](https://issues.apache.org/jira/browse/CALCITE-2303?focusedCommentId=16480420=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16480420). To fix i t the class `org.apache.calcite.avatica.util.DateTimeUtils` should be changed which is presented in both avatica and flink. At the same time different db's provide a little bit different behavior (please have a look at the links below) looks like in case of day of week extraction for Oracle, MySql Sunday = 1, for Postgresql Sunday = 0, for Microsoft SQL Server it depends on property set by user On the other hand there is a standard [ISO8601](https://en.wikipedia.org/wiki/ISO_week_date) which also defines weekday, day of years e.g. This is also a part of [CALCITE-2303](https://issues.apache.org/jira/browse/CALCITE-2303). So my suggestion is within this ticket provide support for all operations which could come from [CALCITE-2303](https://issues.apache.org/jira/browse/CALCITE-2303): `dow`, `decade`, `epoch`, `isodow`, `isoyear`, `microsecond` and `millisecond` However yes you are right it is required to choose what approach for dayOfWeek to use. IMHO the simplest way is to use whatever Calcite/avatica provides At the same time [here](https://issues.apache.org/jira/browse/CALCITE-2303?focusedCommentId=16482767=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16482767) Julian Hyde says that > In short, yes, do whatever PostgreSQL does. So they would like to align the behavior with Postrgresql About different db's day of week So the Postgresql's behavior is described [here ](https://www.postgresql.org/docs/9.1/static/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT) with Sunday = 0 + it supports ISO8601 via `isodow `and `isoyear` the Oracle's behavior is described [here ](https://docs.oracle.com/cd/E37483_01/server.751/es_eql/src/ceql_functions_date_extract.html) with Sunday = 1, so far have not found info about support of 8601 via `extract` while it is via `to_char/to_date` Microsoft SQL Server [allows to set up the first weekday](https://docs.microsoft.com/en-us/sql/t-sql/statements/set-datefirst-transact-sql?view=sql-server-2017) at the same time extraction is done via `datepart` not `extract` MySQL [provides weekday](https://dev.mysql.com/doc/refman/5.5/en/date-and-time-functions.html#function_extract) with Sunday = 1 ---
[GitHub] flink issue #6040: [FLINK-9349][Kafka Connector] KafkaConnector Exception wh...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6040 @tzulitai thank you for your review and comments based on your comments I have a question. Could you please clarify it? You mentioned Flink's `OneShotLatch ` and `CheckedThread ` at the same time in some Kafka connector's tests used `AtomicReference`, `Thread` and etc. (I used one of them as an example while writing my version of the test). Just to be on the sage am I right that `OneShotLatch ` and `CheckedThread ` in tests are more preferable or are there some rules/limitations/whatever? ---
[GitHub] flink issue #6032: [FLINK-9378][Table API & SQL] Improve TableException mess...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6032 @twalthr thank you for your review. To be honest thought about such approach indeed org.apache.flink.api.common.typeinfo.BasicTypeInfo#toString should be changed at least to fix case from here https://issues.apache.org/jira/browse/FLINK-9341?focusedCommentId=16475693=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16475693 . So I have 2 questions: 1. is it ok to change public api behavior here (toString is public)? 2. Should we go through all successors of org.apache.flink.api.common.typeinfo.TypeInformation and if its toString looks like clazz.getSimpleName than change it similar to org.apache.flink.api.java.typeutils.GenericTypeInfo#toString? could you please clarify it and than I could do required corrections ---
[GitHub] flink issue #6040: [FLINK-9349][Kafka Connector] KafkaConnector Exception wh...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6040 @tzulitai, @tedyu thnk you for your review, comments and contribution tips I did updates which includes moving test into AbstractFetcherTest and making it kafka connector version independent Could you please help me a bit? Suddenly the travis build failed on YARNSessionCapacitySchedulerITCase (only on flink travis, on my fork it passed several times). It does not look like result of changes as there is nothing related to yarn. Anyway I tried to investigate it. I found several similar issues on jira however they are closed. Also I downloaded logs mentioned in failed travis job > Uploading to transfer.sh https://transfer.sh/JspTz/24547.10.tar.gz based on them it looks like there was a connectivity issue with one of the ApplicationMaster as log yarn-tests/container_1526608500321_0007_01_01/job-manager.log is full of > 2018-05-18 01:56:49,448 WARN akka.remote.transport.netty.NettyTransport - Remote connection to [null] failed with java.net.ConnectException: Connection refused: travis-job-2a2afdc5-7bf8-4597-946e-16551a5ebbc4/127.0.1.1:43980 2018-05-18 01:56:49,449 WARN akka.remote.ReliableDeliverySupervisor - Association with remote system [akka.tcp://flink@travis-job-2a2afdc5-7bf8-4597-946e-16551a5ebbc4:43980] has failed, address is now gated for [50] ms. Reason: [Association failed with [akka.tcp://flink@travis-job-2a2afdc5-7bf8-4597-946e-16551a5ebbc4:43980]] Caused by: [Connection refused: travis-job-2a2afdc5-7bf8-4597-946e-16551a5ebbc4/127.0.1.1:43980] very strange thing > Remote connection to [null] ---
[GitHub] flink pull request #6040: [FLINK-9349][Kafka Connector] KafkaConnector Excep...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6040#discussion_r189168596
--- Diff:
flink-connectors/flink-connector-kafka-0.9/src/test/java/org/apache/flink/streaming/connectors/kafka/internal/Flink9349Test.java
---
@@ -0,0 +1,207 @@
+/*
+ * 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.streaming.connectors.kafka.internal;
+
+import org.apache.flink.api.common.serialization.SimpleStringSchema;
+import org.apache.flink.core.testutils.MultiShotLatch;
+import org.apache.flink.core.testutils.OneShotLatch;
+import org.apache.flink.metrics.groups.UnregisteredMetricsGroup;
+import org.apache.flink.streaming.api.functions.source.SourceFunction;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaCommitCallback;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaTopicPartition;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaTopicPartitionStateSentinel;
+import org.apache.flink.streaming.runtime.tasks.TestProcessingTimeService;
+import
org.apache.flink.streaming.util.serialization.KeyedDeserializationSchema;
+import
org.apache.flink.streaming.util.serialization.KeyedDeserializationSchemaWrapper;
+
+import org.apache.kafka.clients.consumer.ConsumerRecords;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.junit.Assert.assertFalse;
+import static org.mockito.Mockito.anyLong;
+import static org.powermock.api.mockito.PowerMockito.doAnswer;
+import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
+
+/**
+ * Unit tests for the {@link Flink9349Test}.
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(KafkaConsumerThread.class)
+public class Flink9349Test {
+ @Test
+ public void testConcurrentPartitionsDiscoveryAndLoopFetching() throws
Exception {
+
+ // test data
+ final KafkaTopicPartition testPartition = new
KafkaTopicPartition("test", 42);
+ final Map<KafkaTopicPartition, Long> testCommitData = new
HashMap<>();
+ testCommitData.put(testPartition, 11L);
+
+ // to synchronize when the consumer is in its blocking method
+ final OneShotLatch sync = new OneShotLatch();
+
+ // - the mock consumer with blocking poll calls
+ final MultiShotLatch blockerLatch = new MultiShotLatch();
+
+ KafkaConsumer mockConsumer = mock(KafkaConsumer.class);
+ when(mockConsumer.poll(anyLong())).thenAnswer(new
Answer<ConsumerRecords>() {
+
+ @Override
+ public ConsumerRecords answer(InvocationOnMock
invocation) throws InterruptedException {
+ sync.trigger();
+ blockerLatch.await();
+ return ConsumerRecords.empty();
+ }
+ });
+
+ doAnswer(new Answer() {
+ @Override
+ public Void answer(InvocationOnMock invocation) {
[GitHub] flink pull request #6040: [FLINK-9349][Kafka Connector] KafkaConnector Excep...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6040#discussion_r189038623
--- Diff:
flink-connectors/flink-connector-kafka-0.9/src/test/java/org/apache/flink/streaming/connectors/kafka/internal/Flink9349Test.java
---
@@ -0,0 +1,207 @@
+/*
+ * 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.streaming.connectors.kafka.internal;
+
+import org.apache.flink.api.common.serialization.SimpleStringSchema;
+import org.apache.flink.core.testutils.MultiShotLatch;
+import org.apache.flink.core.testutils.OneShotLatch;
+import org.apache.flink.metrics.groups.UnregisteredMetricsGroup;
+import org.apache.flink.streaming.api.functions.source.SourceFunction;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaCommitCallback;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaTopicPartition;
+import
org.apache.flink.streaming.connectors.kafka.internals.KafkaTopicPartitionStateSentinel;
+import org.apache.flink.streaming.runtime.tasks.TestProcessingTimeService;
+import
org.apache.flink.streaming.util.serialization.KeyedDeserializationSchema;
+import
org.apache.flink.streaming.util.serialization.KeyedDeserializationSchemaWrapper;
+
+import org.apache.kafka.clients.consumer.ConsumerRecords;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.junit.Assert.assertFalse;
+import static org.mockito.Mockito.anyLong;
+import static org.powermock.api.mockito.PowerMockito.doAnswer;
+import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
+
+/**
+ * Unit tests for the {@link Flink9349Test}.
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(KafkaConsumerThread.class)
+public class Flink9349Test {
+ @Test
+ public void testConcurrentPartitionsDiscoveryAndLoopFetching() throws
Exception {
--- End diff --
Do you mean to have such test as a separate method in each Kafka
KafkaXYFetcherTest class?
---
[GitHub] flink pull request #6040: [FLINK-9349][Kafka Connector] KafkaConnector Excep...
Github user snuyanzin commented on a diff in the pull request:
https://github.com/apache/flink/pull/6040#discussion_r189035401
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/AbstractFetcher.java
---
@@ -507,7 +507,7 @@ private void updateMinPunctuatedWatermark(Watermark
nextWatermark) {
SerializedValue<AssignerWithPunctuatedWatermarks>
watermarksPunctuated,
ClassLoader userCodeClassLoader) throws IOException,
ClassNotFoundException {
- List<KafkaTopicPartitionState> partitionStates = new
LinkedList<>();
+ List<KafkaTopicPartitionState> partitionStates = new
CopyOnWriteArrayList<>();
--- End diff --
Yes you are right. A question: is it allowed to specify a link on the issue
comment where it was decided to use CopyOnWriteArrayList? Or is it better to
have explanation in a comment only?
---
[GitHub] flink pull request #6040: [FLINK-9349][Kafka Connector] KafkaConnector Excep...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6040 [FLINK-9349][Kafka Connector] KafkaConnector Exception while fetching from multiple kafka topics *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change *(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)* ## Brief change log fix synchronization issue + test to verify it ## Verifying this change This change added tests and can be verified as follows: via org.apache.flink.streaming.connectors.kafka.internal.Flink9349Test#testConcurrentPartitionsDiscoveryAndLoopFetching ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: ( no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no ) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: ( no ) - The S3 file system connector: ( no) ## Documentation - Does this pull request introduce a new feature? ( no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK-9349_KafkaConnector_Exception_while_fetching_from_multiple_kafka_topics Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6040.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6040 commit 8de5f37549607460659e171f9c3b48d0090383c0 Author: snuyanzin <snuyanzin@...> Date: 2018-05-17T16:12:04Z added test and fix for FLINK-9349 by usage of CopyOnWriteArrayList commit eee524e2d2a86af5252ed939000c12a2604917e9 Author: snuyanzin <snuyanzin@...> Date: 2018-05-17T16:35:10Z fix checkstyle ---
[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/5961 sorry, was closed by mistake reopened ---
[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...
GitHub user snuyanzin reopened a pull request: https://github.com/apache/flink/pull/5961 [FLINK-8255][DataSet API, DataStream API] key expressions on named row types do not work *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change Fix issues related to ClassCastExceptions from Flink-8255 + add more tests ## Brief change log - Usage of casting to TupleTypeInfoBase rather than to TupleTypeInfo as RowTypeInfo is a child of TupleTypeInfoBase but in a different branch in compare with TupleTypeInfo - Add more tests which will fail with ClassCastException without changes from the previous item ## Verifying this change *(Please pick either of the following options)* This change added tests and can be verified as follows: - Added tests that validates that ClassCastException related to Flink-8255 are not happen ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes) - The serializers: (don't know) - The runtime per-record code paths (performance sensitive): (don't know) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK-8255_Key_expressions_on_named_row_types_do_not_work Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5961.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #5961 commit 50eb4ead75ddc22e0eee2bf5a2b9d12c37dcaeb4 Author: snuyanzin <snuyanzin@...> Date: 2018-04-29T18:37:05Z FLINK-8255 Key expressions on named row types do not work Test case from description + 2 more Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo commit 96d569025e21172059bb5b34cf93f3a60b5f0a0e Author: snuyanzin <snuyanzin@...> Date: 2018-04-29T18:47:52Z FLINK-8255 Key expressions on named row types do not work Test case from description + 2 more Resolution
[GitHub] flink pull request #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTR...
GitHub user snuyanzin reopened a pull request: https://github.com/apache/flink/pull/6007 [FLINK-8518][Table API & SQL] Support DOW for EXTRACT dow extraction support implemented + tests *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change *(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)* ## Brief change log *(for example:)* - *The TaskInfo is stored in the blob store on job creation time as a persistent artifact* - *Deployments RPC transmits only the blob storage reference* - *TaskManagers retrieve the TaskInfo from the blob cache* ## Verifying this change *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (100MB)* - *Extended integration test for recovery after master (JobManager) failure* - *Added test that validates that TaskInfo is transferred only once across recoveries* - *Manually verified the change by running a 4 node cluser with 2 JobManagers and 4 TaskManagers, a stateful streaming program, and killing one JobManager and two TaskManagers during the execution, verifying that recovery happens correctly.* ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (yes) - If yes, how is the feature documented? (not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink _FLINK-8518]_Table_API_SQL]_Support_DOW,_EPOCH,_DECADE_for_EXTRACT Alternatively you can review and apply these changes as the patch at: https://github.co
[GitHub] flink pull request #6032: [FLINK-9378][Table API & SQL] Improve TableExcepti...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6032 [FLINK-9378][Table API & SQL] Improve TableException message with TypeName usage *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change *(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)* ## Brief change log messages correction ## Verifying this change *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): ( no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: ( no) - The serializers: ( no) - The runtime per-record code paths (performance sensitive): ( no ) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: ( no ) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK-9378_Improve_TableException_message_with_TypeName_usage Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6032.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6032 commit c896b92852e50eef158cb74df6cfcd8ad226333c Author: snuyanzin <snuyanzin@...> Date: 2018-05-17T09:42:26Z use typename rather than simple name ---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTRACT
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 sorry, closed by mistake, restored ---
[GitHub] flink pull request #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTR...
Github user snuyanzin closed the pull request at: https://github.com/apache/flink/pull/6007 ---
[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...
Github user snuyanzin closed the pull request at: https://github.com/apache/flink/pull/5961 ---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW for EXTRACT
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 yes, done ---
[GitHub] flink issue #6007: [FLINK-8518][Table API & SQL] Support DOW, EPOCH, DECADE ...
Github user snuyanzin commented on the issue: https://github.com/apache/flink/pull/6007 @walterddr thank you for your comment it is just DOW implementation because to implement EPOCH we have to wait for CALCITE-2303 resolution about DECADE - there are 2 ways: one is also to wait for CALCITE-2303 another one is to do the same changes in _org.apache.calcite.avatica.util.DateTimeUtils#julianExtract_ (Flink has its own version of this class) as proposed in CALCITE-2303 and then do other stuff in Flink ---
[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...
Github user snuyanzin commented on the issue:
https://github.com/apache/flink/pull/5961
Hello @fhueske
Thank you for your review
As you proposed I tried to use
org.apache.flink.api.java.typeutils.RowTypeInfo#isTupleType
```java
public boolean isTupleType() {
return false;
}
```
however after that these tests started to fail
org.apache.flink.table.api.batch.ExplainTest#testJoinWithoutExtended
org.apache.flink.table.api.batch.ExplainTest#testJoinWithExtended
like
```
testJoinWithoutExtended(org.apache.flink.table.api.batch.ExplainTest) Time
elapsed: 0.037 sec <<< ERROR!
org.apache.flink.api.common.InvalidProgramException: Specifying keys via
field positions is only valid for tuple data types. Type: Row(a: Integer, b:
String)
at
org.apache.flink.api.common.operators.Keys$ExpressionKeys.(Keys.java:232)
at
org.apache.flink.api.common.operators.Keys$ExpressionKeys.(Keys.java:223)
at
org.apache.flink.api.java.operators.JoinOperator$JoinOperatorSets.where(JoinOperator.java:901)
at
org.apache.flink.table.plan.nodes.dataset.DataSetJoin.addInnerJoin(DataSetJoin.scala:243)
at
org.apache.flink.table.plan.nodes.dataset.DataSetJoin.translateToPlan(DataSetJoin.scala:170)
at
org.apache.flink.table.plan.nodes.dataset.DataSetCalc.translateToPlan(DataSetCalc.scala:91)
at
org.apache.flink.table.api.BatchTableEnvironment.translate(BatchTableEnvironment.scala:422)
at
org.apache.flink.table.api.BatchTableEnvironment.explain(BatchTableEnvironment.scala:249)
at
org.apache.flink.table.api.BatchTableEnvironment.explain(BatchTableEnvironment.scala:275)
at
org.apache.flink.table.api.batch.ExplainTest.testJoinWithoutExtended(ExplainTest.scala:72)
```
that is why I decided to use instanceof TupleTypeInfo check as anyway next
line there is a cast to this type
could you please have a look at it and tell if it is acceptable or not?
---
[GitHub] flink pull request #6007: [FLINK-8518][Table API & SQL] Support DOW, EPOCH, ...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/6007 [FLINK-8518][Table API & SQL] Support DOW, EPOCH, DECADE for EXTRACT dow extraction support implemented + tests *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change *(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)* ## Brief change log *(for example:)* - *The TaskInfo is stored in the blob store on job creation time as a persistent artifact* - *Deployments RPC transmits only the blob storage reference* - *TaskManagers retrieve the TaskInfo from the blob cache* ## Verifying this change *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (100MB)* - *Extended integration test for recovery after master (JobManager) failure* - *Added test that validates that TaskInfo is transferred only once across recoveries* - *Manually verified the change by running a 4 node cluser with 2 JobManagers and 4 TaskManagers, a stateful streaming program, and killing one JobManager and two TaskManagers during the execution, verifying that recovery happens correctly.* ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (yes) - If yes, how is the feature documented? (not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink _FLINK-8518]_Table_API_SQL]_Support_DOW,_EPOCH,_DECADE_for_EXTRACT Alternatively you can review and apply these changes as the patch at: http
[GitHub] flink pull request #5961: [Flink-8255][DataSet API, DataStream API] key expr...
GitHub user snuyanzin opened a pull request: https://github.com/apache/flink/pull/5961 [Flink-8255][DataSet API, DataStream API] key expressions on named row types do not work *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.* *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.* ## Contribution Checklist - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue. - Name the pull request in the form "[FLINK-] [component] Title of the pull request", where *FLINK-* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component. Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`. - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review. - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices). - Each pull request should address only one issue, not mix up code from multiple issues. - Each commit in the pull request has a meaningful commit message (including the JIRA id) - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below. **(The sections below can be removed for hotfixes of typos)** ## What is the purpose of the change Fix issues related to ClassCastExceptions from Flink-8255 + add more tests ## Brief change log - Usage of casting to TupleTypeInfoBase rather than to TupleTypeInfo as RowTypeInfo is a child of TupleTypeInfoBase but in a different branch in compare with TupleTypeInfo - Add more tests which will fail with ClassCastException without changes from the previous item ## Verifying this change *(Please pick either of the following options)* This change added tests and can be verified as follows: - Added tests that validates that ClassCastException related to Flink-8255 are not happen ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes) - The serializers: (don't know) - The runtime per-record code paths (performance sensitive): (don't know) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/snuyanzin/flink FLINK-8255_Key_expressions_on_named_row_types_do_not_work Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5961.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #5961 commit 50eb4ead75ddc22e0eee2bf5a2b9d12c37dcaeb4 Author: snuyanzin <snuyanzin@...> Date: 2018-04-29T18:37:05Z FLINK-8255 Key expressions on named row types do not work Test case from description + 2 more Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo commit 96d569025e21172059bb5b34cf93f3a60b5f0a0e Author: snuyanzin <snuyanzin@...> Date: 2018-04-29T18:47:52Z FLINK-8255 Key expressions on named row types do not work Test case from description + 2 more Resolution of class cast by usin
