Skip to content

Conversation

@smola
Copy link
Contributor

@smola smola commented May 28, 2015

  • ExpressionBuilders is provided with helpers to create a function builder
    for each Expression.
  • Built-in functions removed from SqlParser when possible. Added to
    FunctionRegistry.

TO DO:

  • Decide between the reflection and macro implementations of the
    expression builder helpers.
  • Fix Substring (whose constructor is not well suited for the helper).
  • Apply changes to Hive.

- ExpressionBuilders is provided with helpers to create a function builder
  for each Expression.
- Built-in functions removed from SqlParser when possible. Added to
  FunctionRegistry.

TO DO:

- Decide between the reflection and macro implementations of the
  expression builder helpers.
- Fix Substring (whose constructor is not well suited for the helper).
- Apply changes to Hive.
@smola
Copy link
Contributor Author

smola commented May 28, 2015

This is still a work in progress.

@rxin I would like some feedback about how to continue with the implementation. In ExpressionBuilders I added some helper methods that can convert a Expression into a function name and a function builder (i.e. (Seq[Expression]) => Expression). I added two alternative implementations, one with macros and one with reflection. How does it look?

Any other feedback will be really appreciated. Thanks.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@smola
Copy link
Contributor Author

smola commented May 28, 2015

I'm working on a simpler solution without macros or reflection involved.

@rxin
Copy link
Contributor

rxin commented May 28, 2015

Oh yes definitely prefer the no-macro way, since it is doable without macros. Thanks!

@rxin
Copy link
Contributor

rxin commented May 30, 2015

Ping me when you think this is ready for review. Thanks!

rxin added a commit to rxin/spark that referenced this pull request Jun 8, 2015
[SPARK-7886][SQL] Add built-in expressions to FunctionRegistry.
@rxin
Copy link
Contributor

rxin commented Jun 8, 2015

@smola I needed this functionality so I pulled yours in, fixed some small things, and pushed a new version here: #6710

asfgit pushed a commit that referenced this pull request Jun 9, 2015
This patch switches to using FunctionRegistry for built-in expressions. It is based on #6463, but with some work to simplify it along with unit tests.

TODOs for future pull requests:
- Use static registration so we don't need to register all functions every time we start a new SQLContext
- Switch to using this in HiveContext

Author: Reynold Xin <rxin@databricks.com>
Author: Santiago M. Mola <santi@mola.io>

Closes #6710 from rxin/udf-registry and squashes the following commits:

6930822 [Reynold Xin] Fixed Python test.
b802c9a [Reynold Xin] Made UDF case insensitive.
e60d815 [Reynold Xin] Made UDF case insensitive.
852f9c0 [Reynold Xin] Fixed style violation.
e76a3c1 [Reynold Xin] Fixed parser.
52ddaba [Reynold Xin] Fixed compilation.
ee7854f [Reynold Xin] Improved error reporting.
ff906f2 [Reynold Xin] More robust constructor calling.
77b46f1 [Reynold Xin] Simplified the code.
2a2a149 [Reynold Xin] Merge pull request #6463 from smola/SPARK-7886
8616924 [Santiago M. Mola] [SPARK-7886] Add built-in expressions to FunctionRegistry.
@smola
Copy link
Contributor Author

smola commented Jun 11, 2015

@rxin Thank you! I'm closing this PR.

@smola smola closed this Jun 11, 2015
@smola smola deleted the SPARK-7886 branch June 11, 2015 09:00
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
This patch switches to using FunctionRegistry for built-in expressions. It is based on apache#6463, but with some work to simplify it along with unit tests.

TODOs for future pull requests:
- Use static registration so we don't need to register all functions every time we start a new SQLContext
- Switch to using this in HiveContext

Author: Reynold Xin <rxin@databricks.com>
Author: Santiago M. Mola <santi@mola.io>

Closes apache#6710 from rxin/udf-registry and squashes the following commits:

6930822 [Reynold Xin] Fixed Python test.
b802c9a [Reynold Xin] Made UDF case insensitive.
e60d815 [Reynold Xin] Made UDF case insensitive.
852f9c0 [Reynold Xin] Fixed style violation.
e76a3c1 [Reynold Xin] Fixed parser.
52ddaba [Reynold Xin] Fixed compilation.
ee7854f [Reynold Xin] Improved error reporting.
ff906f2 [Reynold Xin] More robust constructor calling.
77b46f1 [Reynold Xin] Simplified the code.
2a2a149 [Reynold Xin] Merge pull request apache#6463 from smola/SPARK-7886
8616924 [Santiago M. Mola] [SPARK-7886] Add built-in expressions to FunctionRegistry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants