Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

null not working in q/mq filter in subscriptions #2998

Closed
fgalan opened this issue Oct 5, 2017 · 8 comments
Closed

null not working in q/mq filter in subscriptions #2998

fgalan opened this issue Oct 5, 2017 · 8 comments
Assignees
Labels
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Oct 5, 2017

Situation:

Having this subscription in place:

   {
        "id": "59d675c5b473c61d5aa876f3",
        "notification": {
            "attrs": [
                "t"
            ],
            "attrsFormat": "normalized",
            "http": {
                "url": "http://10.x.y.z:10031/notify"
            }
        },
        "status": "active",
        "subject": {
            "condition": {
                "attrs": [
                    "t"
                ],
                "expression": {
                    "q": "t==null"
                }
            },
            "entities": [
                {
                    "idPattern": ".*"
                }
            ]
        }
    }

And creating this entity:

{
        "id": "E1",
        "type": "T",
        "t": {
            "metadata": {},
            "type": "StructuredData",
            "value": null
        }
}

Bug:

  • Creating that entity doesn't triggers notification (i.e. timesSent is not present in subscription). Changing from/to null not triggers notification.
@fgalan fgalan added the bug label Oct 5, 2017
@fgalan
Copy link
Member Author

fgalan commented Oct 5, 2017

This is related with functionality developed in #2359. Looking to cases/2359... the null_values_in_string_filters.test doesn't covers any subscription case.

@fgalan fgalan added this to the 1.9.0 milestone Oct 5, 2017
@fgalan
Copy link
Member Author

fgalan commented Oct 5, 2017

It seems the same problem occurs with mq and metadata filters.

@fgalan fgalan changed the title Quotes not working in q filter in subscriptions Quotes not working in q/mq filter in subscriptions Oct 5, 2017
@fgalan fgalan modified the milestones: 1.9.0, 1.10.0 Oct 19, 2017
@fgalan fgalan changed the title Quotes not working in q/mq filter in subscriptions null not working in q/mq filter in subscriptions Oct 20, 2017
@kzangeli
Copy link
Member

First impression:
we probably don't distinguish between 'not present' and 'present but with NULL value'.
To distinguish between the two cases we'd need a boolean field 'present in payload', which I believe we don't have, unfortunately ...

Also first impression: Shouldn't be too hard to fix this problem.

@fgalan
Copy link
Member Author

fgalan commented Oct 30, 2017

I don't remember this pretty well, but I think we implemented a type selector for attributes and metadata:

typedef enum ValueType
{
  ValueTypeUnknown,
  ValueTypeString,
  ValueTypeNumber,
  ValueTypeBoolean,
  ValueTypeVector,
  ValueTypeObject,
  ValueTypeNone
} ValueType;

ValueTypeNone is the one used for null.

Although maybe the lack of distintion between "not present" and "present but with NULL value" you refer is in the filter object, not in the attribute/metadata value, and this comment is pointless :)

@fgalan
Copy link
Member Author

fgalan commented Nov 10, 2017

Could be related with #2587

@fgalan
Copy link
Member Author

fgalan commented Nov 10, 2017

As discussed at skype, maybe is a matter of going from:

typedef enum ValueType
{
  ValueTypeUnknown,
  ValueTypeString,
  ValueTypeNumber,
  ValueTypeBoolean,
  ValueTypeVector,
  ValueTypeObject,
  ValueTypeNone
} ValueType;

to

typedef enum ValueType
{
  ValueTypeUnknown,
  ValueTypeString,
  ValueTypeNumber,
  ValueTypeBoolean,
  ValueTypeVector,
  ValueTypeObject,
  ValueTypeNull,      // old ValueTypeNone
  ValueTypeNotGiven   // new, to mean cases in which the value was not provided
} ValueType;

In order to resolve currenta ambiguety with ValueTypeNone to deal with both situations in the code.

@fgalan
Copy link
Member Author

fgalan commented Nov 15, 2017

Fixed in PR #3039

However, I'm not going to close the issue yet. The fix is that PR is not definitive and I'll like to have a look to how this stuff is implemented (I'm assigning to me).

@fgalan
Copy link
Member Author

fgalan commented Nov 20, 2017

Refactor done in PR #3046. That PR removes the #if temporal solution. Thus, I understand that PR provides a final solution and this issue could be closed.

@fgalan fgalan closed this as completed Nov 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants