sqltyper icon indicating copy to clipboard operation
sqltyper copied to clipboard

Published 20 hours ago • verified publisher icon akheron

First attempt at supporting functionCall in FROM

Open FrigoEU opened this issue 3 years ago • 4 comments

Hi,

As mentioned in #123, this is my first attempt at supporting function calls in FROM clauses.

I changed the infer module a bit to support both SourceColumn and VirtualField in a bunch of places. I think this will kinda work, but it definitely has some limitations:

  • Only functions that return (arrays of) a single value are supported. Functions that return records aren't supported.
  • Overloaded functions probably aren't handled correctly either.

Very first try, so I'm happy to get any feedback at all!

FrigoEU avatar Mar 23 '21 12:03 FrigoEU

Thanks for the PR!

I think we should first add support for just unnest with one literal parameter, i.e. to just support your use case. Otherwise this rabbit hole is going to be too deep for one PR or even two :smile:

In this case, you could just return an array of one column from getSourceColumnsForTableExpr, and there would be no need to call inferExpressionNullability at all. Calling it doesn't really make sense in this case, because the function call evaluates to a table.

I think using SourceColumn would be more appropriate than VirtualField, which is really meant for output columns. You could change the type of tableAlias to string | undefined and see which parts break. This would also avoid the uglyish hack of isSourceColumn type guard.

akheron avatar Mar 23 '21 17:03 akheron

@FrigoEU do you plan to work on this or wanna me to take over?

akheron avatar Mar 27 '21 06:03 akheron

Hey,

I was planning to come back to this at some point. I'm not sure I agree that using Virtualfield's is not OK. It works well, and I also think it's functionally correct. But if you really don't agree, then I'll change it as you suggested :).

Simon

On Sat, 27 Mar 2021, 07:04 Petri Lehtinen, @.***> wrote:

@FrigoEU https://github.com/FrigoEU do you plan to work on this or wanna me to take over?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/akheron/sqltyper/pull/124#issuecomment-808665563, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCBXAX2FM4FOJFXV6C2L2TTFVYPNANCNFSM4ZVD7YLA .

FrigoEU avatar Mar 27 '21 07:03 FrigoEU

Well, I don’t have a very strong opinion. I just noticed all the SourceColumn | VirtualField in the diff and thought that it’s not optimal.

akheron avatar Mar 27 '21 13:03 akheron