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

Kusto functions support #2572

Merged
merged 16 commits into from
Oct 13, 2024
Merged

Kusto functions support #2572

merged 16 commits into from
Oct 13, 2024

Conversation

barnuri
Copy link
Contributor

@barnuri barnuri commented Oct 9, 2024

in GetParameters the sql methods type_name, OdbcPrec, OdbcScale, OBJECT_NAME, OBJECT_SCHEMA_NAME not supported

in GetResultElementLists the sql methods type_name, OBJECT_ID is not supported

replace all by join with relevant tables

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Does Kusto have functions and stored procedures??

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

If not, then we should just add a check for Kusto in the scaffolder, and not attempt to scaffold procedures and functions if we are running against Kusto

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

And mayb revert Kusto related changes in these classes?

@barnuri
Copy link
Contributor Author

barnuri commented Oct 9, 2024

Does Kusto have functions and stored procedures??

no, they have function table, kusto views is function without params, also they have function table with params, so i need the function table support

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Wonder if we can discover them - at any reate, we should just refarin from trying to scaffold stored procedure, so changes to SqlServerRoutineModelFactory.cs are not needed, we need to add a check in ReverseEngineerRunner instead

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Have you managed to smoke test this with Kusto AND a SQL database with functions?

@barnuri
Copy link
Contributor Author

barnuri commented Oct 9, 2024

Have you managed to smoke test this with Kusto AND a SQL database with functions?

managed to run e2e on kusto, and run the specific query on sql server online emulator (doesnt have real sql server)

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

@barnuri You can spin up AdventureWorks on an Azure SQL database, it has some SQL functions

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

@barnuri I will try this PR branch against Adventureworks (or another database with Functions and stored procedures)

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Just tested with Adventureworks, and getting errors - I am reluctant to accept this PR, the queries should be left alone for non_Kusto usage, suggest that a variation for Kusto is used instead.

@barnuri
Copy link
Contributor Author

barnuri commented Oct 9, 2024

@barnuri You can spin up AdventureWorks on an Azure SQL database, it has some SQL functions

looks fine :)
image
image

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Can you find accountnumber ?

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2024

Regardsless, there are too many changes to these two queries, that have worked well and contain a lot of knowledge

@barnuri
Copy link
Contributor Author

barnuri commented Oct 9, 2024

accountnumber

image

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 10, 2024

These two queries return wildliy dfferent results against AdventureWorks:

SELECT  
    'Parameter' = p.name,  
    'Type'   = COALESCE(ts.name, tu.name),
    'Length'   = CAST(p.max_length AS INT),  
    'Precision'   = CASE 
              WHEN ts.name = 'uniqueidentifier' THEN p.precision  
              WHEN ts.name IN ('decimal', 'numeric') THEN p.precision
              WHEN ts.name IN ('varchar', 'nvarchar') THEN p.max_length
              ELSE NULL
            END, 
    'Scale'   = CAST(p.scale AS INT),  
    'Order'  = CAST(p.parameter_id AS INT),  
    p.is_output AS output,
    'TypeName' = QUOTENAME(s.name) + '.' + QUOTENAME(tu.name),
	'TypeSchema' = tu.schema_id,
	'TypeId' = p.user_type_id,
    'RoutineName' = o.name,
    'RoutineSchema' = s.name
    from sys.parameters p
    inner join sys.objects AS o on o.object_id = p.object_id
    inner JOIN sys.schemas AS s ON o.schema_id = s.schema_id
    left JOIN sys.types AS tu ON p.user_type_id = tu.user_type_id
    left JOIN sys.types AS ts ON p.system_type_id = ts.system_type_id
    ORDER BY p.object_id, p.parameter_id;
	SELECT  
    'Parameter' = p.name,  
    'Type'   = COALESCE(type_name(p.system_type_id), type_name(p.user_type_id)),  
    'Length'   = CAST(p.max_length AS INT),  
    'Precision'   = CAST(case when type_name(p.system_type_id) = 'uniqueidentifier' 
                then p.precision  
                else OdbcPrec(p.system_type_id, p.max_length, p.precision) end AS INT),  
    'Scale'   = CAST(OdbcScale(p.system_type_id, p.scale) AS INT),  
    'Order'  = CAST(parameter_id AS INT),  
    p.is_output AS output,
    'TypeName' = QUOTENAME(SCHEMA_NAME(t.schema_id)) + '.' + QUOTENAME(TYPE_NAME(p.user_type_id)),
	'TypeSchema' = t.schema_id,
	'TypeId' = p.user_type_id,
    'RoutineName' = OBJECT_NAME(p.object_id),
    'RoutineSchema' = OBJECT_SCHEMA_NAME(p.object_id)
    from sys.parameters p
	LEFT JOIN sys.table_types t ON t.user_type_id = p.user_type_id
    ORDER BY p.object_id, p.parameter_id;

@barnuri
Copy link
Contributor Author

barnuri commented Oct 10, 2024

These two queries return wildliy dfferent results against AdventureWorks:

SELECT  
    'Parameter' = p.name,  
    'Type'   = COALESCE(ts.name, tu.name),
    'Length'   = CAST(p.max_length AS INT),  
    'Precision'   = CASE 
              WHEN ts.name = 'uniqueidentifier' THEN p.precision  
              WHEN ts.name IN ('decimal', 'numeric') THEN p.precision
              WHEN ts.name IN ('varchar', 'nvarchar') THEN p.max_length
              ELSE NULL
            END, 
    'Scale'   = CAST(p.scale AS INT),  
    'Order'  = CAST(p.parameter_id AS INT),  
    p.is_output AS output,
    'TypeName' = QUOTENAME(s.name) + '.' + QUOTENAME(tu.name),
	'TypeSchema' = tu.schema_id,
	'TypeId' = p.user_type_id,
    'RoutineName' = o.name,
    'RoutineSchema' = s.name
    from sys.parameters p
    inner join sys.objects AS o on o.object_id = p.object_id
    inner JOIN sys.schemas AS s ON o.schema_id = s.schema_id
    left JOIN sys.types AS tu ON p.user_type_id = tu.user_type_id
    left JOIN sys.types AS ts ON p.system_type_id = ts.system_type_id
    ORDER BY p.object_id, p.parameter_id;
	SELECT  
    'Parameter' = p.name,  
    'Type'   = COALESCE(type_name(p.system_type_id), type_name(p.user_type_id)),  
    'Length'   = CAST(p.max_length AS INT),  
    'Precision'   = CAST(case when type_name(p.system_type_id) = 'uniqueidentifier' 
                then p.precision  
                else OdbcPrec(p.system_type_id, p.max_length, p.precision) end AS INT),  
    'Scale'   = CAST(OdbcScale(p.system_type_id, p.scale) AS INT),  
    'Order'  = CAST(parameter_id AS INT),  
    p.is_output AS output,
    'TypeName' = QUOTENAME(SCHEMA_NAME(t.schema_id)) + '.' + QUOTENAME(TYPE_NAME(p.user_type_id)),
	'TypeSchema' = t.schema_id,
	'TypeId' = p.user_type_id,
    'RoutineName' = OBJECT_NAME(p.object_id),
    'RoutineSchema' = OBJECT_SCHEMA_NAME(p.object_id)
    from sys.parameters p
	LEFT JOIN sys.table_types t ON t.user_type_id = p.user_type_id
    ORDER BY p.object_id, p.parameter_id;

do you have suggestions ?
from looking in the results is look like the new query return things that we didnt found before maybe its ok ?

maybe we can do if kusto use the new query ? what do you think ?

@barnuri
Copy link
Contributor Author

barnuri commented Oct 10, 2024

fixed the first sql

SELECT * INTO #temp1 FROM (
SELECT 
    c.name,
    COALESCE(ts.name, tu.name) AS type_name,
    c.column_id AS column_ordinal,
    c.is_nullable
FROM sys.columns c
inner join sys.types tu ON c.user_type_id = tu.user_type_id 
inner join sys.objects AS o on o.object_id = c.object_id
inner JOIN sys.schemas AS s ON o.schema_id = s.schema_id
LEFT JOIN sys.types ts ON tu.system_type_id = ts.user_type_id
) as a

SELECT * INTO #temp2 FROM (
SELECT 
    c.name,
    COALESCE(type_name(c.system_type_id), type_name(c.user_type_id)) AS type_name,
    c.column_id AS column_ordinal,
    c.is_nullable
FROM sys.columns c

) as b

-- Compare the results
SELECT * FROM #temp1
EXCEPT 
SELECT * FROM #temp2;

-- Compare in the reverse direction to check if both results are identical
SELECT * FROM #temp2 
EXCEPT 
SELECT * FROM #temp1;

@barnuri
Copy link
Contributor Author

barnuri commented Oct 10, 2024

@ErikEJ the first query return the same
for the second added if kusto

@barnuri barnuri requested a review from ErikEJ October 10, 2024 12:50
@ErikEJ
Copy link
Owner

ErikEJ commented Oct 10, 2024

Leaving this until the feedback on the EF Core pr has been addressed

@barnuri
Copy link
Contributor Author

barnuri commented Oct 10, 2024

Leaving this until the feedback on the EF Core pr has been addressed

how it's connected ? the only left comment there is on something that i didnt changed here

@barnuri
Copy link
Contributor Author

barnuri commented Oct 13, 2024

Leaving this until the feedback on the EF Core pr has been addressed

update all the places from the prev PR too like i fixed in EF Core pr, found a solution with less changes the efcore team probably will like

validate the change with efcpt.8 with kusto (smoking test)

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2024

Cool, will have a look soon

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2024

Have you rebased on latest master?

@barnuri
Copy link
Contributor Author

barnuri commented Oct 13, 2024

Have you rebased on latest master?

yeah, i sync fork master and then pull fork master to this branch

Copy link
Owner

@ErikEJ ErikEJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a couple of changes needed

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets revert any changes to this file, the EF Core 6 tools does not support Kusto anyway, and I will remove EF 6 support soon anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, reverted

@barnuri barnuri requested a review from ErikEJ October 13, 2024 18:01
@ErikEJ ErikEJ merged commit 9232a3d into ErikEJ:master Oct 13, 2024
2 checks passed
@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2024

Thanks @barnuri for a great contribution

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2024

@barnuri any plans to share this via a blog post or similar??

@barnuri barnuri deleted the kusto-functions-support branch October 13, 2024 19:39
@barnuri
Copy link
Contributor Author

barnuri commented Oct 13, 2024

@barnuri any plans to share this via a blog post or similar??

never done that before, if you have an example of something or write by your blog its great too

also can you update here ? i think they waiting for your response

dotnet/efcore#34832

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.

2 participants