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

Function namespaces should influence the schema function entries get inserted into #2371

Open
scsmithr opened this issue Jan 8, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@scsmithr
Copy link
Member

scsmithr commented Jan 8, 2024

Description

Followup from #2368

Function names and function entries in the catalog are disjointed right now. When building the builtin catalog, all functions are iterated over, and placed into the default schema (public).

This is particularly troublesome with array_to_string since it's both a datafusion function, and an alias for postgres compatability (pg_catalog.array_to_string). However, the existing logic dumps pg_catalog.array_to_string into the default schema with the name array_to_string, completely disregarding the function namespace.

The function namespace should be used when determining which schema to place functions in. I think the easiest thing to do would be move namespace from BuiltinScalarUDF to BuiltinFunction, then get the namespace when building the catalog to get the appropriate schema.

@scsmithr scsmithr added the bug Something isn't working label Jan 8, 2024
scsmithr added a commit that referenced this issue Jan 8, 2024
Closes #2368

Futzing around with function aliases. This replaces `HashMap` with
`AliasMap` to allow multiple keys to point to the same object. Also adds
some tests.

I needed to add a workaround for 'array_to_string' for one of the
iterators, see #2371.
@universalmind303 universalmind303 added this to the next milestone Jan 11, 2024
@universalmind303
Copy link
Contributor

we should probably hold off on this until we do #2490.

That issue will likely result in a large refactor of our function registry anyways, so it'd probably be easiest to do them both in a single pass.

@tychoish tychoish removed this from the next milestone Feb 1, 2024
@tychoish
Copy link
Contributor

tychoish commented Feb 1, 2024

[pulled out of priority queue, because it's a polish thing and not blocking anything so it's safe to defer at the moment.]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants