You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by rposkocil <gi...@git.apache.org> on 2017/01/04 23:39:00 UTC
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
GitHub user rposkocil opened a pull request:
https://github.com/apache/metamodel/pull/139
Support for NOT / NOT IN operator
fixes #METAMODEL-1134
https://issues.apache.org/jira/browse/METAMODEL-1134
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rposkocil/metamodel feature/1134-condition-negation-causes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metamodel/pull/139.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 #139
----
commit 766a7fbf98084392e16c3559ad157a07dcb824ae
Author: rposkocil <r....@gmc.net>
Date: 2017-01-04T22:33:37Z
Possible to negate the filter condition
- added filter attribute for negation
- added new negation operator
- added test of it
commit 2d6f1d0060a22619c98e1f7c3046cd8d5eb94fb1
Author: rposkocil <r....@gmc.net>
Date: 2017-01-04T23:35:45Z
New NOT IN operator
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
Yes that sounds correct to me. I think this is much more manageable for existing implementations which typically do something like this:
```
if (operator == EQUALS_TO) {
return somethingUsefull();
} else if (operator == LIKE) {
return somethingElseUsefull();
} else {
// this is not supported - return null to make MetaModel evaluate the filter item on the client side.
return null;
}
```
(the above snippet would still be valid even though other operators are added. If we added the negation option, it would not).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/139#discussion_r95316556
--- Diff: core/src/main/java/org/apache/metamodel/query/builder/AbstractFilterBuilder.java ---
@@ -64,6 +64,21 @@ public B in(String... strings) {
}
@Override
+ public B not_in(Collection<?> values) {
--- End diff --
We should keep using regular java method names here (and below), so `notIn`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by rposkocil <gi...@git.apache.org>.
Github user rposkocil commented on the issue:
https://github.com/apache/metamodel/pull/139
Prepared by the accepted way.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/139#discussion_r95316577
--- Diff: core/src/main/java/org/apache/metamodel/query/builder/AbstractFilterBuilder.java ---
@@ -460,6 +475,15 @@ public B like(String string) {
}
@Override
+ public B not_like(String string) {
--- End diff --
and `notLike` here
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
Your PR suggests to solve the problem in two ways, and that seems to me to be one too many. One way is with the `NegationOperator` which is in a way nice and general, but introduces some redundancy since there are also operators available for "different from". And it is made more obvious with the other solution that you introduce at the same time: Introducing the "not in" operator. Honestly I think that from an API evolution point of view, the second approach ("not in" operator) is the best. Introducing a new generic concept like negation into FilterItem is going to have a lot more side-effects on existing query evaluation. I would just add this one new operator and not handle negation generically.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by rposkocil <gi...@git.apache.org>.
Github user rposkocil commented on the issue:
https://github.com/apache/metamodel/pull/139
I was thinking about it and for a complete option to be able to express all, what propositional logic allows, it should be enough so that we will add NOT IN and NOT LIKE operators. All operators should have a positive and negative form then every conditions can be transfomed to allowed constructions.
The next point is that I have forgoten on the fluent api for queries where the operators have to impletented, too.
Do you agree with this solution?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
LGTM!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/139#discussion_r95316508
--- Diff: core/src/main/java/org/apache/metamodel/query/OperatorTypeImpl.java ---
@@ -75,40 +75,44 @@ public static OperatorType convertOperatorType(String sqlType) {
if (sqlType != null) {
sqlType = sqlType.trim().toUpperCase();
switch (sqlType) {
- case "=":
- case "==":
- case "EQ":
- case "EQUALS_TO":
- return OperatorType.EQUALS_TO;
- case "<>":
- case "!=":
- case "NE":
- case "NOT_EQUAL":
- case "NOT_EQUAL_TO":
- case "NOT_EQUALS":
- case "NOT_EQUALS_TO":
- case "DIFFERENT_FROM":
- return OperatorType.DIFFERENT_FROM;
- case ">":
- case "GT":
- case "GREATER_THAN":
- return OperatorType.GREATER_THAN;
- case ">=":
- case "=>":
- case "GREATER_THAN_OR_EQUAL":
- return OperatorType.GREATER_THAN_OR_EQUAL;
- case "IN":
- return OperatorType.IN;
- case "<":
- case "LT":
- case "LESS_THAN":
- return OperatorType.LESS_THAN;
- case "<=":
- case "=<":
- case "LESS_THAN_OR_EQUAL":
- return OperatorType.LESS_THAN_OR_EQUAL;
- case "LIKE":
- return OperatorType.LIKE;
+ case "=":
+ case "==":
+ case "EQ":
+ case "EQUALS_TO":
+ return OperatorType.EQUALS_TO;
+ case "<>":
+ case "!=":
+ case "NE":
+ case "NOT_EQUAL":
+ case "NOT_EQUAL_TO":
+ case "NOT_EQUALS":
+ case "NOT_EQUALS_TO":
+ case "DIFFERENT_FROM":
+ return OperatorType.DIFFERENT_FROM;
+ case ">":
+ case "GT":
+ case "GREATER_THAN":
+ return OperatorType.GREATER_THAN;
+ case ">=":
+ case "=>":
+ case "GREATER_THAN_OR_EQUAL":
+ return OperatorType.GREATER_THAN_OR_EQUAL;
+ case "NOT IN":
+ return OperatorType.NOT_IN;
+ case "IN":
+ return OperatorType.IN;
+ case "<":
+ case "LT":
+ case "LESS_THAN":
+ return OperatorType.LESS_THAN;
+ case "<=":
+ case "=<":
+ case "LESS_THAN_OR_EQUAL":
+ return OperatorType.LESS_THAN_OR_EQUAL;
+ case "LIKE":
+ return OperatorType.LIKE;
+ case "NOT_LIKE":
--- End diff --
We should definately add "NOT LIKE" here also (with space instead of underscore).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
Applied the patch to master. Thank you very much @rposkocil and sorry for the long wait (another time you can bump it if you want to reinvigorate our attention ;-))
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
Alternatively we should deprecate DIFFERENT_FROM and not introduce NOT_IN. That would technically speaking be fine. But my last argument is that from an evolution perspective, it would be worse, because it forces a lot of existing implementations to evaluate a new attribute which was not there before. It would implicitly introduce a ton of bugs because data contexts out there would no longer execute queries correctly.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by rposkocil <gi...@git.apache.org>.
Github user rposkocil commented on the issue:
https://github.com/apache/metamodel/pull/139
Can you see any other weird things or issues, @kaspersorensen ? Thank you.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/139
Yes but having two ways of doing it means that we can now also have "NOT NOT_IN" (same as IN) and "NOT DIFFERENT_FROM" (same as EQUALS_TO) which to me seems like we've done our design wrong. I agree that ideally we would then just have the negation option, but that train has already left the station since we have negations built in to DIFFERENT_FROM. I also wonder why you would want to introduce NOT_IN if you really think we should be using the negation option instead?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel issue #139: Support for NOT / NOT IN operator
Posted by rposkocil <gi...@git.apache.org>.
Github user rposkocil commented on the issue:
https://github.com/apache/metamodel/pull/139
Correct me, please @kaspersorensen, but NegationOperator is much more general than different from and additing another way to propositional logic of filters. Operator Different from is used just for childs of filters and it's represented with <>. For example SQL NOT keyword has a wider usage. You can negate also composed filters on all levels or different from, too. This functionality I didn't found there and thought that it could be fine.
There is no way to negate IN construct in the currecnt version of MetaModel. That's why Hans created this requirement. There can be IN and NOT IN operators like = and <>. A similiar case.
So both parts make me sense and don't brake the concept. But I understand that there could be other problems that's why constructors were doubled with negationa and without.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by rposkocil <gi...@git.apache.org>.
Github user rposkocil commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/139#discussion_r95653591
--- Diff: core/src/main/java/org/apache/metamodel/query/OperatorTypeImpl.java ---
@@ -75,40 +75,44 @@ public static OperatorType convertOperatorType(String sqlType) {
if (sqlType != null) {
sqlType = sqlType.trim().toUpperCase();
switch (sqlType) {
- case "=":
- case "==":
- case "EQ":
- case "EQUALS_TO":
- return OperatorType.EQUALS_TO;
- case "<>":
- case "!=":
- case "NE":
- case "NOT_EQUAL":
- case "NOT_EQUAL_TO":
- case "NOT_EQUALS":
- case "NOT_EQUALS_TO":
- case "DIFFERENT_FROM":
- return OperatorType.DIFFERENT_FROM;
- case ">":
- case "GT":
- case "GREATER_THAN":
- return OperatorType.GREATER_THAN;
- case ">=":
- case "=>":
- case "GREATER_THAN_OR_EQUAL":
- return OperatorType.GREATER_THAN_OR_EQUAL;
- case "IN":
- return OperatorType.IN;
- case "<":
- case "LT":
- case "LESS_THAN":
- return OperatorType.LESS_THAN;
- case "<=":
- case "=<":
- case "LESS_THAN_OR_EQUAL":
- return OperatorType.LESS_THAN_OR_EQUAL;
- case "LIKE":
- return OperatorType.LIKE;
+ case "=":
+ case "==":
+ case "EQ":
+ case "EQUALS_TO":
+ return OperatorType.EQUALS_TO;
+ case "<>":
+ case "!=":
+ case "NE":
+ case "NOT_EQUAL":
+ case "NOT_EQUAL_TO":
+ case "NOT_EQUALS":
+ case "NOT_EQUALS_TO":
+ case "DIFFERENT_FROM":
+ return OperatorType.DIFFERENT_FROM;
+ case ">":
+ case "GT":
+ case "GREATER_THAN":
+ return OperatorType.GREATER_THAN;
+ case ">=":
+ case "=>":
+ case "GREATER_THAN_OR_EQUAL":
+ return OperatorType.GREATER_THAN_OR_EQUAL;
+ case "NOT IN":
+ return OperatorType.NOT_IN;
+ case "IN":
+ return OperatorType.IN;
+ case "<":
+ case "LT":
+ case "LESS_THAN":
+ return OperatorType.LESS_THAN;
+ case "<=":
+ case "=<":
+ case "LESS_THAN_OR_EQUAL":
+ return OperatorType.LESS_THAN_OR_EQUAL;
+ case "LIKE":
+ return OperatorType.LIKE;
+ case "NOT LIKE":
--- End diff --
Ok, sorry for all mistakes
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metamodel/pull/139
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metamodel pull request #139: Support for NOT / NOT IN operator
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/139#discussion_r95523550
--- Diff: core/src/main/java/org/apache/metamodel/query/OperatorTypeImpl.java ---
@@ -75,40 +75,44 @@ public static OperatorType convertOperatorType(String sqlType) {
if (sqlType != null) {
sqlType = sqlType.trim().toUpperCase();
switch (sqlType) {
- case "=":
- case "==":
- case "EQ":
- case "EQUALS_TO":
- return OperatorType.EQUALS_TO;
- case "<>":
- case "!=":
- case "NE":
- case "NOT_EQUAL":
- case "NOT_EQUAL_TO":
- case "NOT_EQUALS":
- case "NOT_EQUALS_TO":
- case "DIFFERENT_FROM":
- return OperatorType.DIFFERENT_FROM;
- case ">":
- case "GT":
- case "GREATER_THAN":
- return OperatorType.GREATER_THAN;
- case ">=":
- case "=>":
- case "GREATER_THAN_OR_EQUAL":
- return OperatorType.GREATER_THAN_OR_EQUAL;
- case "IN":
- return OperatorType.IN;
- case "<":
- case "LT":
- case "LESS_THAN":
- return OperatorType.LESS_THAN;
- case "<=":
- case "=<":
- case "LESS_THAN_OR_EQUAL":
- return OperatorType.LESS_THAN_OR_EQUAL;
- case "LIKE":
- return OperatorType.LIKE;
+ case "=":
+ case "==":
+ case "EQ":
+ case "EQUALS_TO":
+ return OperatorType.EQUALS_TO;
+ case "<>":
+ case "!=":
+ case "NE":
+ case "NOT_EQUAL":
+ case "NOT_EQUAL_TO":
+ case "NOT_EQUALS":
+ case "NOT_EQUALS_TO":
+ case "DIFFERENT_FROM":
+ return OperatorType.DIFFERENT_FROM;
+ case ">":
+ case "GT":
+ case "GREATER_THAN":
+ return OperatorType.GREATER_THAN;
+ case ">=":
+ case "=>":
+ case "GREATER_THAN_OR_EQUAL":
+ return OperatorType.GREATER_THAN_OR_EQUAL;
+ case "NOT IN":
+ return OperatorType.NOT_IN;
+ case "IN":
+ return OperatorType.IN;
+ case "<":
+ case "LT":
+ case "LESS_THAN":
+ return OperatorType.LESS_THAN;
+ case "<=":
+ case "=<":
+ case "LESS_THAN_OR_EQUAL":
+ return OperatorType.LESS_THAN_OR_EQUAL;
+ case "LIKE":
+ return OperatorType.LIKE;
+ case "NOT LIKE":
--- End diff --
Sorry, I meant to add this, not replace the old, so ...
```
case "NOT_LIKE":
case "NOT LIKE":
...
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---