[GitHub] phoenix pull request #222: PHOENIX-3355 Register Phoenix built-in functions ...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/222 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix issue #224: PHOENIX-3477 Support sequence arithmetic in Calcite-Phoe...
Github user lomoree commented on the issue: https://github.com/apache/phoenix/pull/224 After discussing options, we kept original design and merged as is. Closing PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #224: PHOENIX-3477 Support sequence arithmetic in Calci...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/224 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #224: PHOENIX-3477 Support sequence arithmetic in Calci...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/224#discussion_r91162501
--- Diff:
phoenix-core/src/it/java/org/apache/phoenix/calcite/CalciteIT.java ---
@@ -1343,6 +1343,28 @@ public void initTable() throws Exception {
{7L, "05", "T5", "05", "S5"},
{8L, "06", "T6", "06", "S6"}})
.close();
+
--- End diff --
While that was my initial thinking, upon looking deeper I discovered a
different source for the issue
The reason a ServerProject was being created was because of visitCall(). It
wasn't recursively checking for sequences. Therefore, when an operator like
SqlKind.PLUS was present and the sequence was present, the sequence would be
pushed down to a child operand and not be seen by visitCall()
PhoenixConverterRules is designed so that a ServerProject does not get
created if a sequence is present
```
private static Predicate NO_SEQUENCE = new
Predicate() {
@Override
public boolean apply(LogicalProject input) {
return !CalciteUtils.hasSequenceValueCall(input);
}
};
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #224: PHOENIX-3477 Support sequence arithmetic in Calci...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/224 PHOENIX-3477 Support sequence arithmetic in Calcite-Phoenix PhoenixConverterRules is meant to route any logical project through PhoenixClientProject. Before, sequences were not properly being found. This patch simplifies the logic and adds that support. -- Check for nested sequences -- Use the Sequence Manager now available in StatementContext -- Add relevant test cases to CalciteIT You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix sequencearithmetic Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/224.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 #224 commit 8eacb9e455f16c97d6f301fdb1c27494905900a0 Author: Eric Lomore <elom...@bloomberg.net> Date: 2016-12-03T01:00:20Z PHOENIX-3477 Support sequence arithmetic in Calcite-Phoenix --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #223: PHOENIX-3488 Support COUNT(DISTINCT x) in Phoenix...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/223 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #223: PHOENIX-3488 Support COUNT(DISTINCT x) in Phoenix...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/223#discussion_r90563623
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -899,9 +899,6 @@ public static boolean isConvertible(Aggregate input) {
if (input.getGroupSets().size() > 1)
return false;
-if (input.containsDistinctCall())
--- End diff --
@maryannxue added a commit to detect for any distinct calls that aren't of
type COUNT. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #222: PHOENIX-3355 Register Phoenix built-in functions as Calc...
Github user lomoree commented on the issue:
https://github.com/apache/phoenix/pull/222
1. No differentiation between the regex functions (String based and Byte
based)
Yep I'll try some approaches to make this work.
2. Invert and MD5 Function don't work (phoenix doesn't supply the correct
arguments in annotation)
Might be complicated. Phoenix uses a generic "term" as the input argument
for these functions.
See https://phoenix.apache.org/language/index.html#term
We don't really have a type that encapsulates this kind of behavior as far
as I know.
This type of argument is represented by the following annotation:
@BuiltInFunction(name = InvertFunction.NAME, args = { @Argument() })
3. Timezone parameter doesn't work properly in ToDateFunction (basic cases
work, but custom timezones do not act as expected)
I believe that's correct. When I was doing testing all of the results are
inexplicably off by an offset that is exactly the difference between UTC and
and EST :)
4. Type matching doesn't work on input arguments (date/timestamp)
I explored this after our meeting. This used to be an issue in Calcite but
was patched about a year ago with the corresponding test cases added. When
debugging these test cases, I never touched the typePrecedence block that's an
issue for us. Looks like I'll need to investigate more.
5. Array constructors are not supported (another issue entirely)
Yes, I did make a jira for it, but perhaps we can expand it as you said.
https://issues.apache.org/jira/browse/PHOENIX-3502
6. Code refactoring:
Having them in one spot is starting to make more sense; in that case, I
agree that PhoenixSchema would be the best location for everything. What I can
do is having them ordered according to how they logically progress, and have a
comment block at the top that describes the behavior.
In my opinion it's appropriate and reasonable to include 1, 3, and 6 in
this pull request. The other issues may take longer, and would postpone
integrating this component.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix issue #222: PHOENIX-3355 Register Phoenix built-in functions as Calc...
Github user lomoree commented on the issue: https://github.com/apache/phoenix/pull/222 1. There are still a number of cases that don't pass. Many of them are due to existing features not being supported, and many are due to other issues within Calcite-Phoenix. I have compiled a list of the major common issues though: - No differentiation between the regex functions (String based and Byte based) - Invert and MD5 Function don't work (phoenix doesn't supply the correct arguments in annotation) - Type matching doesn't work on input arguments (date/timestamp) - Timezone parameter doesn't work properly in ToDateFunction (basic cases work, but custom timezones do not act as expected) - Array constructors are not supported (another issue entirely) 2. I believe you're referring to those in PhoenixScalarFunction, createBuiltinFunctions() and evaluateReturnType()? In the case of createBuiltinFunctions() it makes sense to be in PhoenixScalarFunction since it create instances of that class. We could move evaluateReturnType() to PhoenixSchema since it has more generic behavior, but I'm not entirely convinced that it should move. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #223: PHOENIX-3488 Support COUNT(DISTINCT x) in Phoenix...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/223 PHOENIX-3488 Support COUNT(DISTINCT x) in Phoenix-Calcite Integration You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix countdistinct Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/223.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 #223 commit f3feb0ac314b36a99faf3684b43c7e2fde0d44d8 Author: Eric <elom...@bloomberg.net> Date: 2016-11-16T19:48:18Z Support COUNT(DISTINCT x) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #222: PHOENIX-3355 Register Phoenix built-in functions ...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/222 PHOENIX-3355 Register Phoenix built-in functions as Calcite functions This patch works for nearly all built in functions. Aggregate functions are not yet supported. Ultimately we may want to rework: - The use of parse nodes as a factory - Generic return types to explicit return types These issues are hand in hand as abstract built in functions (NowFunction, FloorFunction) are where the parse node factory & generic return types are necessary. Potential resolution from my point of view is 2 parts - Create a link somewhere between an abstract function and its concrete implementations (maybe require inheritance, create an annotation that links the abstract function to its implementations, or include the logic in the below mentioned factory) - Create a new factory that replaces the use of parse nodes You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix builtinfunctions Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/222.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 #222 commit 3f5d65ced67db31d2cd638e66a2ff0f93ae07e5a Author: Eric <elom...@bloomberg.net> Date: 2016-10-20T19:49:56Z Builtin functions initial commit commit 524423220e90205c40748f4b9e3339ad8e4ba315 Author: Eric <elom...@bloomberg.net> Date: 2016-10-31T17:49:13Z Argument overloading support commit 53cc70317414f72650b3cff1649fbcf0ad5edf25 Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-11-02T20:14:01Z Integration tests commit b0cd8fa3f8229a54074d3d38e05b4ce09b6c99b1 Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-11-09T21:44:17Z Use parsenode as a factory commit 6c94c2290521d11313edf64e930ba9f687ed7861 Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-11-09T22:30:10Z Generic return types - temp fix --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #220: PHOENIX-3394 Handle SequenceResolving through Con...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/220 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #220: PHOENIX-3394 Handle SequenceResolving through Con...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/220#discussion_r86365551
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/PhoenixSchema.java ---
@@ -329,24 +333,14 @@ private TableRef fixTableMultiTenancy(TableRef
tableRef) throws SQLException {
private PhoenixSequence resolveSequence(String name) {
try {
-// FIXME: Do this the same way as resolving a table after
PHOENIX-2489.
-String tenantId = pc.getTenantId() == null ? null :
pc.getTenantId().getString();
-String q = "select 1 from " +
PhoenixDatabaseMetaData.SYSTEM_SEQUENCE
-+ " where " + PhoenixDatabaseMetaData.SEQUENCE_SCHEMA
-+ (schemaName == null ? " is null" : " = '" +
schemaName + "'")
-+ " and " + PhoenixDatabaseMetaData.SEQUENCE_NAME
-+ " = '" + name + "'"
-+ " and " + PhoenixDatabaseMetaData.TENANT_ID
-+ (tenantId == null ? " is null" : " = '" + tenantId +
"'");
-ResultSet rs = pc.createStatement().executeQuery(q);
-if (rs.next()) {
-return new PhoenixSequence(schemaName, name, pc);
-}
-} catch (SQLException e) {
-throw new RuntimeException(e);
+SequenceManager manager = new
SequenceManager((PhoenixStatement)pc.createStatement());
+manager.newSequenceReference(pc.getTenantId(),
TableName.createNormalized(schemaName, name) , null,
SequenceValueParseNode.Op.NEXT_VALUE);
--- End diff --
Good question. From my testing, passing in null doesn't work. A static
member is an option if nothing else.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #220: PHOENIX-3394 Handle SequenceResolving through Con...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/220#discussion_r86365234
--- Diff:
phoenix-core/src/it/java/org/apache/phoenix/calcite/CalciteIT.java ---
@@ -1315,6 +1315,17 @@ public void initTable() throws Exception {
{6L, "00A323122312312"},
{8L, "00A423122312312"}})
.close();
+
--- End diff --
Calcite seems to function a little bit differently with respect to CURRENT
VALUE, so I thought it would be wise to include a case for it. Plus it's the
only case in CalciteIT covering CURRENT VALUE.
See: https://issues.apache.org/jira/browse/PHOENIX-3437
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #220: PHOENIX-3394 Handle SequenceResolving through Con...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/220 PHOENIX-3394 Handle SequenceResolving through ConnectionQueryServices interface You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix sequenceresolving Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/220.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 #220 commit 5ab71200e7d9df42efd3eacbc63383da810cce2e Author: Eric <elom...@bloomberg.net> Date: 2016-10-28T22:23:49Z Sequence resolving initial fix commit b987c51e0538d498f2c5a15b18d22411a6674134 Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-11-03T02:39:36Z Clean up & integration tests --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #219: PHOENIX-3267 Replace use of SELECT null with CAST...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/219 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #219: PHOENIX-3267 Replace use of SELECT null with CAST...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/219 PHOENIX-3267 Replace use of SELECT null with CAST(null AS ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/219.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 #219 commit d8aaa60bd49aa0fc29d5f4e97288199f18d56b1b Author: Eric Lomore <elom...@bloomberg.net> Date: 2016-10-25T00:04:40Z PHOENIX-3267 Replace use of SELECT null with CAST(null AS ) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #215: PHOENIX-3345 SQLException code's not propagating ...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/215 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #218: PHOENIX-3265 Surround columns named date with dou...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/218 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #218: PHOENIX-3265 Surround columns named date with dou...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/218 PHOENIX-3265 Surround columns named date with double quotes -- Intended to make phoenix compatible with calcite. Calcite has reserved keywords such as date and dec which must be surrounded with quotes. You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/218.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 #218 commit 3ba4f1977e81b5da1c4b9717a07036a0ae7bc520 Author: Eric <elom...@bloomberg.net> Date: 2016-10-05T18:55:59Z PHOENIX-3265 Surround columns named date with double quotes --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #215: PHOENIX-3345 SQLException code's not propagating ...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/215#discussion_r83024234
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1120,8 +1120,11 @@ public static SQLException
unwrapSqlException(SQLException root){
Exception e = root;
while(e.getCause() != null){
e = (Exception) e.getCause();
+if(e instanceof RuntimeException && e.getCause() instanceof
SQLException) {
--- End diff --
Latest changes attempt to locate the nested SQLException within an
RuntimeException. We still throw the first SQLException seen in case there's no
wrapping argument exception. And if no matches are found, we throw the root so
as to avoid unexpected behavior.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #216: PHOENIX-3265 Surround columns named date with dou...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/216 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #215: PHOENIX-3345 SQLException code's not propagating ...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/215#discussion_r82853555
--- Diff:
phoenix-core/src/main/java/org/apache/calcite/jdbc/PhoenixCalciteFactory.java
---
@@ -161,6 +162,29 @@ public Object apply(
}));
}
+public CalciteStatement createStatement(String sql, int
resultSetType, int resultSetConcurrency, int resultSetHoldability) throws
SQLException {
+try {
+return super.createStatement(resultSetType,
resultSetConcurrency, resultSetHoldability);
+} catch (SQLException e) {
+if (e.getCause().getCause() instanceof SQLException) {
+throw (SQLException) e.getCause().getCause();
--- End diff --
The new changes go for the deepest SQL exception. Can quickly change it if
you would like only the next most deep SQL exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #215: PHOENIX-3345 SQLException code's not propagating ...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/215#discussion_r82688259
--- Diff:
phoenix-core/src/main/java/org/apache/calcite/jdbc/PhoenixCalciteFactory.java
---
@@ -161,6 +162,29 @@ public Object apply(
}));
}
+public CalciteStatement createStatement(String sql, int
resultSetType, int resultSetConcurrency, int resultSetHoldability) throws
SQLException {
+try {
+return super.createStatement(resultSetType,
resultSetConcurrency, resultSetHoldability);
+} catch (SQLException e) {
+if (e.getCause().getCause() instanceof SQLException) {
+throw (SQLException) e.getCause().getCause();
--- End diff --
I wondered about this myself. While I didn't encounter any cases where
exception levels varied, it's a good idea to account for it. Will make these
changes, thanks @maryannxue
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #214: PHOENIX-3298 Single column primary key may not be...
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/214 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #214: PHOENIX-3298 Single column primary key may not be...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/214#discussion_r82305850
--- Diff:
phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java ---
@@ -1453,7 +1453,7 @@ public void testInvalidPrimaryKeyDecl() throws
Exception {
}
}
}
-
+
--- End diff --
Of course, I will avoid fixing these in the future. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #216: PHOENIX-3265 Surround columns named date with dou...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/216 PHOENIX-3265 Surround columns named date with double quotes -- Only a few additional unit tests passed -- Major impact on IT tests. Cherry-picked commit onto master to ensure no glaring errors, but difficult to test on calcite branch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix reservedkeywords Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/216.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 #216 commit 3f1c926c7604df4abd3a24c95e1870f4c26f8bc0 Author: Eric <elom...@bloomberg.net> Date: 2016-10-05T18:55:59Z PHOENIX-3265 Surround columns named date with double quotes --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #215: PHOENIX-3345 SQLException code's not propagating ...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/215 PHOENIX-3345 SQLException code's not propagating as expected -- Primarily impacts unit tests in QueryCompilerTest.java You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix exceptions Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/215.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 #215 commit a2c8bdf051773ff83dc6bb94ffcfd1edf2efd13b Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-10-06T21:01:39Z PHOENIX-3345 SQLException code's not propogating as expected --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree closed the pull request at: https://github.com/apache/phoenix/pull/213 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #214: PHOENIX-3298 Single column primary key may not be...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/214 PHOENIX-3298 Single column primary key may not be null Note: This patch will not immediately resolve the testInvalidPrimaryKeyDecl() test case due to another issue, PHOENIX-3345. However, they are independent issues so there's no reason to wait on this. You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix nullcolumns Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/214.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 #214 commit 669c000c8bea3230f2e895cefebbd00cf6f7d7c6 Author: ERIC LOMORE <elom...@bloomberg.net> Date: 2016-09-30T06:19:18Z PHOENIX-3298 Single column primary key may not be null --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81431821
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
Ah, my mistake. I'll get working on a patch that also supports offset
without limit. This should incorporate all of the changes noted above. Thanks
for clarifying.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81422631
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -232,8 +232,7 @@ public RelNode convert(RelNode rel) {
private static Predicate HAS_FETCH = new
Predicate() {
@Override
public boolean apply(LogicalSort input) {
-return input.offset == null
-&& input.fetch != null;
+return input.fetch != null;
--- End diff --
With respect to my comment below, if there is a limit, then we apply the
rule, otherwise we do not (regardless of offset).
I may have the wrong idea here, but based on my understanding of offset,
all of the test cases pass as expected. Are the test cases I have laid out in
CalciteIT what you would expect?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81421343
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
This does definitely have ripple effects throughout, but I had done this on
the basis that SQL only allows an offset if their is an imposed limit (since an
offset doesn't make any logical sense without a limit).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/213 PHOENIX-2827 Support OFFSET in Calcite-Phoenix You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix offset Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/213.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 #213 commit 488259067d5a610efcc946cb92d48070aa4fbdb0 Author: Eric <elom...@bloomberg.net> Date: 2016-09-28T21:40:32Z PHOENIX-2827 Support OFFSET in Calcite-Phoenix --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81216671
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
+try {
+final SchemaPlus rootSchema =
Frameworks.createRootSchema(true);
+final FrameworkConfig config = Frameworks.newConfigBuilder()
+.parserConfig(SqlParser.Config.DEFAULT)
+.defaultSchema(rootSchema).build();
+Planner planner = Frameworks.getPlanner(config);
+
+SqlParserPos POS = SqlParserPos.ZERO;
+final SqlNodeList selectList =
+new SqlNodeList(
+Collections.singletonList(literal),
+SqlParserPos.ZERO);
+
+
+String sql = new SqlSelect(POS, SqlNodeList.EMPTY, selectList,
null, null, null, null,
+SqlNodeList.EMPTY, null, null, null).toString();
+SqlNode sqlNode = planner.parse(sql);
+sqlNode = planner.validate(sqlNode);
+Project proj = (Project) (planner.rel(sqlNode).rel);
+RexNode rex = proj.getChildExps().get(0);
+
+Expression e = CalciteUtils.toExpression(rex, implementor);
+ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+e = ExpressionUtil.getConstantExpression(e, ptr);
+Object ret = e.getDataType().toObject(ptr);
+if(ret instanceof NlsString){
--- End diff --
Yes I meant that as well. I think it's because we had originally intended
to use Calcite utilities to do this. I just ran through a debugger and all
inputs parse down to either a Char or a String. It is definitely covered by
test cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81212299
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
+try {
+final SchemaPlus rootSchema =
Frameworks.createRootSchema(true);
+final FrameworkConfig config = Frameworks.newConfigBuilder()
+.parserConfig(SqlParser.Config.DEFAULT)
+.defaultSchema(rootSchema).build();
+Planner planner = Frameworks.getPlanner(config);
+
+SqlParserPos POS = SqlParserPos.ZERO;
+final SqlNodeList selectList =
+new SqlNodeList(
+Collections.singletonList(literal),
+SqlParserPos.ZERO);
+
+
+String sql = new SqlSelect(POS, SqlNodeList.EMPTY, selectList,
null, null, null, null,
+SqlNodeList.EMPTY, null, null, null).toString();
+SqlNode sqlNode = planner.parse(sql);
+sqlNode = planner.validate(sqlNode);
+Project proj = (Project) (planner.rel(sqlNode).rel);
+RexNode rex = proj.getChildExps().get(0);
+
+Expression e = CalciteUtils.toExpression(rex, implementor);
+ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+e = ExpressionUtil.getConstantExpression(e, ptr);
+Object ret = e.getDataType().toObject(ptr);
+if(ret instanceof NlsString){
+ret = ((NlsString) ret).toString();
+}
+return ret;
+}
+catch (Exception e){
+throw new RuntimeException("Could not convert literal to its
object type: " + e);
--- End diff --
Will amend this, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81212263
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
+try {
+final SchemaPlus rootSchema =
Frameworks.createRootSchema(true);
+final FrameworkConfig config = Frameworks.newConfigBuilder()
+.parserConfig(SqlParser.Config.DEFAULT)
+.defaultSchema(rootSchema).build();
+Planner planner = Frameworks.getPlanner(config);
+
+SqlParserPos POS = SqlParserPos.ZERO;
+final SqlNodeList selectList =
+new SqlNodeList(
+Collections.singletonList(literal),
+SqlParserPos.ZERO);
+
+
+String sql = new SqlSelect(POS, SqlNodeList.EMPTY, selectList,
null, null, null, null,
+SqlNodeList.EMPTY, null, null, null).toString();
+SqlNode sqlNode = planner.parse(sql);
+sqlNode = planner.validate(sqlNode);
+Project proj = (Project) (planner.rel(sqlNode).rel);
+RexNode rex = proj.getChildExps().get(0);
+
+Expression e = CalciteUtils.toExpression(rex, implementor);
+ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+e = ExpressionUtil.getConstantExpression(e, ptr);
+Object ret = e.getDataType().toObject(ptr);
+if(ret instanceof NlsString){
--- End diff --
I'm not sure if it's necessary, but decided to use it per James'
recommendation. He might have some insight on why it's needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81210767
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
+try {
+final SchemaPlus rootSchema =
Frameworks.createRootSchema(true);
+final FrameworkConfig config = Frameworks.newConfigBuilder()
+.parserConfig(SqlParser.Config.DEFAULT)
+.defaultSchema(rootSchema).build();
+Planner planner = Frameworks.getPlanner(config);
--- End diff --
I could also abstract this to the SqlOptionNode and pass it in to this
method. I had that at one point but ended up opting for this since the code
didn't seem like it belonged there. Let me know which you prefer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81210455
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
--- End diff --
Agreed, will update.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/212#discussion_r81210093
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/CalciteUtils.java ---
@@ -1036,4 +1048,40 @@ public Void visitCall(RexCall call) {
return null;
}
}
+
+public static Object convertLiteral(SqlLiteral literal,
PhoenixRelImplementor implementor) {
+try {
+final SchemaPlus rootSchema =
Frameworks.createRootSchema(true);
+final FrameworkConfig config = Frameworks.newConfigBuilder()
+.parserConfig(SqlParser.Config.DEFAULT)
+.defaultSchema(rootSchema).build();
+Planner planner = Frameworks.getPlanner(config);
+
+SqlParserPos POS = SqlParserPos.ZERO;
+final SqlNodeList selectList =
+new SqlNodeList(
+Collections.singletonList(literal),
+SqlParserPos.ZERO);
+
+
+String sql = new SqlSelect(POS, SqlNodeList.EMPTY, selectList,
null, null, null, null,
--- End diff --
I tried desperately to do this, but Calcite has pretty strict enforcement
on Sql States. When the node is created its in STATE 1: READY, and it will not
allow you to go straight to a validation state. Is there some way of overriding
this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #212: PHOENIX-3264 Allow TRUE and FALSE to be used as l...
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/212 PHOENIX-3264 Allow TRUE and FALSE to be used as literal constants You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix calcite Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/212.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 #212 commit fb59ed04b151fd8e7533cf5c99c70118a697de77 Author: Eric <elom...@bloomberg.net> Date: 2016-09-28T17:56:29Z PHOENIX-3264 Allow TRUE and FALSE to be used as literal constants --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
