[GitHub] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
+1 by inspection.  Great work, otto.


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
It would make a nice little POC -


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
The annotation would have to be made more complex.  We can accept varargs 
as part of the function, for instance.  I think it's possible, but perhaps we 
would need to iterate a bit on the annotation.


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
Yeah agreed.  I don't think we should be executing the function as part of 
validate.  Perhaps just ensuring the function resolves.  Maybe expanding the 
interface for stellar function to provide a validate method so it's pluggable 
by function.


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
ok, as above, I'm not sure about x->null to do that, but I am the noob 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[Casey Stella]

The purpose of validate is to ensure the statement makes syntactic sense
from a stellar perspective and it's only called when statements change in
the config.
On Thu, Oct 6, 2016 at 21:15 Casey Stella  wrote:

> Oh no validate is not called at runtime for every packet! That's called at
> stellar statement input time (e.g. Config pushes to zookeeper)
> On Thu, Oct 6, 2016 at 21:12 ottobackwards  wrote:
>
> Github user ottobackwards commented on the issue:
>
> https://github.com/apache/incubator-metron/pull/293
>
> I took this jira to get some start of an idea about Stellar, and after
> debugging through it to track down to exitVariable to find the argument
> resolution and then back up to the validate x->null I would say I got what
> I bargained for.
>
> I am not sure how I would answer your question on validation, I don't
> know Stellar well enough.  My experience and intuition tells me that
> running the 'execute' and using an exception or error case as validation is
> not very efficient at runtime, if in fact this is what happens at runtime
> as well.  Better to have real metadata on the function and just validating
> the metadata and they syntax of the query I would think.   *A
> logical/runtime error does not mean an invalid expression*.  This method is
> in effect equating them which I think is incorrect ( unless I am mistaking
> the intent of validate()).
>
>
>
>
> ---
> 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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
Sorry was responding on the dev list, not this PR.  The purpose of validate 
is to ensure syntactic correctness from a stellar point of view.  This is not 
called per message, but only when stellar statements change (e.g. At config 
push to zookeeper time)


---
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.
---

Re: [GitHub] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[Casey Stella]

Oh no validate is not called at runtime for every packet! That's called at
stellar statement input time (e.g. Config pushes to zookeeper)
On Thu, Oct 6, 2016 at 21:12 ottobackwards  wrote:

> Github user ottobackwards commented on the issue:
>
> https://github.com/apache/incubator-metron/pull/293
>
> I took this jira to get some start of an idea about Stellar, and after
> debugging through it to track down to exitVariable to find the argument
> resolution and then back up to the validate x->null I would say I got what
> I bargained for.
>
> I am not sure how I would answer your question on validation, I don't
> know Stellar well enough.  My experience and intuition tells me that
> running the 'execute' and using an exception or error case as validation is
> not very efficient at runtime, if in fact this is what happens at runtime
> as well.  Better to have real metadata on the function and just validating
> the metadata and they syntax of the query I would think.   *A
> logical/runtime error does not mean an invalid expression*.  This method is
> in effect equating them which I think is incorrect ( unless I am mistaking
> the intent of validate()).
>
>
>
>
> ---
> 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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
@mmiklavc I had seen that.  The tests in StellarTests test the compiler and 
the variable resolution etc as well is my point.  It is worth having both.  If 
both were done from the start, maybe  METRON-439 doesn't get through


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
I took this jira to get some start of an idea about Stellar, and after 
debugging through it to track down to exitVariable to find the argument 
resolution and then back up to the validate x->null I would say I got what I 
bargained for.

I am not sure how I would answer your question on validation, I don't know 
Stellar well enough.  My experience and intuition tells me that running the 
'execute' and using an exception or error case as validation is not very 
efficient at runtime, if in fact this is what happens at runtime as well.  
Better to have real metadata on the function and just validating the metadata 
and they syntax of the query I would think.   *A logical/runtime error does not 
mean an invalid expression*.  This method is in effect equating them which I 
think is incorrect ( unless I am mistaking the intent of validate()).




---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[mmiklavc]

Github user mmiklavc commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
@ottobackwards A unit test has been added as part of PR#296 -> 
https://github.com/mmiklavc/incubator-metron/blob/bd6e1a5cc48962af36b131e189ffb461e836cd20/metron-platform/metron-common/src/test/java/org/apache/metron/common/dsl/functions/DataStructureFunctionsTest.java


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
So, yes, a couple of things need to be ensured for stellar functions:
* The output of the call is serializable via kryo (the profiler stores the 
output of stellar statements in HBase)
* It passes the `validate` call

Validate is intended to check syntactic errors and it does so by passing a 
VariableResolver that returns null for every variable.  I will admit that I've 
been thinking this should be rethought and we should do this another way:
* Have a special implementation of StellarBaseListener that barfs if 
function resolution happens, but doesn't actually apply functions
* Uses the normal Stellar Lexer to ensure syntactic errors
This should ensure that the stellar statement is valid syntactically IMO.

Thoughts?


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
OK I have it in.  Here is the thing that I found,  I know there is a pull 
about fixing IS_EMPTY from throwing exceptions - and I had to take that same 
return 0 approach instead of throwing.  IS_EMPTY did NOT have a test in 
StellarTests, so it was never being tested through the stellar variable 
resolver, validation, parser etc.  If it had had a test there ( using the run() 
as implemented in the test ) it never would have been committed, because it 
NEVER works, no matter what you pass it as originally implemented with the 
IllegalStateExceptions.  This is because the validate calls down to the apply 
with a specifically null argument which triggers the exception.
So, a couple of things:

- every stellar function needs to be tested through the whole stellar 
framework and not just on top of StellarFunction.apply()
- why would validate call parse which calls apply?  Is that the way it is 
at runtime?  Does apply always get called twice?  Is the test harness wrong?


---
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] incubator-metron issue #293: METRON-473 Add LENGTH() To Stellar

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/incubator-metron/pull/293
  
@cestella I can do that.  Should I move LENGTH to DataStructureFunctions 
then?
Also - I notice that IS_EMPTY throws IllegalStateExceptions instead of 
returning null, is that how no-var should be handled?  In the String functions 
they return 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.
---