SQL stored procs for labels to replace python procs

[joshelser]

• Split up label tests into separate files

[joshelser]

Non-scientific results show create label latency on the low end around 500ms and on the upper end around 2.5s. I would imagine we can chop some of that off by consolidating the number of "validation rows". I wasn't focused on that in this pass (more focused on correctness and maintainable tests).

[joshelser]

Jacques mentioned that he wanted to take a look at this change, so I've tagged him for review.

[joshelser]

FWIW, there wasn't much in terms of python tests we added that weren't already encapsulated in the label test cases (largely, because we didn't write pytests that required Snowflake -- just exercising the pydantic validations).

[jacques-n]

Hey, can you separate this into two PRs?

PR1: Moving runtime validation logic from python to SQL. PR2: Separating python tests into multiple files (with virtually no code changes).

[joshelser]

Pushed a few commits:

• admin.write(..) now incrementally builds up the "pieces" from the incoming object, to then combine into the skeleton of the MERGE statement. This is much more readable to me.
• Tried to capture the obvious validation (is not, is not null) as UDFs that generate the condition.
• I changed over the validations for ungrouped labels to creating a single object, and updated admin.validate to use those.

I didn't migrate the grouped labels and dynamic labels validations (so those tests will fail). I wanted to see what you two thought.

[joshelser]

@rymurr feedback from friday. Notably, I dropped grouped and dynamic label stuff from the current PR to reduce the size of it. One addition that we didn't discuss was that I also dropped the overloaded CREATE_LABEL and UPDATE_LABEL procedures (making is_dynamic default false lets us avoid the same blocks of code twice).

The UPDATE_LABEL procedure is still complicated -- I feel like the clean way to address complexity there is to make separate procedures for other labels, rather than overloading into singular procedures.

[joshelser]

Any comments on time and improvements?

Yeesh, we took a step backwards for ungrouped label creation

ADMIN.CREATE_LABEL() is taking ~6s. Of that, the current breakdown is:

• admin.validate takes ~3.2s
• admin.write takes ~1.2s
• internal.update_label_view takes ~1.8s

The sum of queries executed by admin.write are around 700ms, so 1.2s seems long. The two "complex" validations represent ~50% of the total 3.2s of validation. I'll see if there's something obvious we can do differently here.

I'll re-evaluate with a hybrid table in a momen.t

[joshelser]

I'll re-evaluate with a hybrid table in a moment

Making internal.labels a hybrid table with label_id primary key drops the execution time of the MERGE statement inside admin.write from ~400ms to ~250ms.

[jacques-n]

I think we should be able to make all validation a single SQL select query. Prove me wrong :D

[joshelser]

I think we should be able to make all validation a single SQL select query. Prove me wrong :D

I just pushed the latest (which has grouped and ungrouped labels passing their tests, but not dynamic labels). I did a quick prototype with just the simple validations in one select but ignored the "complex" validations (which we know for sure are the time-suck).

I'll see if I can get all of the checks into a single query. The one problem I can immediately think of is that we rely on an exception to be raised to verify that a label name doesn't duplicate a name in enriched_query_history. Will have to take a different approach on that check.

[rymurr]

Maybe an unpopular opinion: should we give this up and just go back to V1 of these? V1 beign specialized procedures for each crud type? I seem to recall those being faster than this. My memory:

v1: First we had standalone hand coded procs for each entity type. Reasonably fast but hard to extend and had to be called as stored procs from streamlit v2: Move into a generic python structure. Easy to extend and very fast from streamlit but slow from stored procs v3: since we are getting rid of streamlit we don't care if its fast from streamlit. We need fast from stored procs. Do we need it to be easy to extend?

My suggestion, in the interest of expediency is to either defer this whole problem or to stop trying to make generic validation. Thoughts?

[joshelser]

Maybe an unpopular opinion: should we give this up and just go back to V1 of these? V1 beign specialized procedures for each crud type?

I had the same thought yesterday. I almost installed an old version just to confirm that they were actually faster before.