-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: add QUERY_TECHNICAL_MARK to all UI queries #1992
Conversation
ed0fdd5
to
6a1ff44
Compare
6a1ff44
to
59f2ae3
Compare
@@ -63,27 +65,28 @@ function createShardQueryHistorical( | |||
PeakTime, | |||
InFlightTxCount, | |||
IntervalEnd | |||
FROM \`.sys/top_partitions_one_hour\` | |||
FROM \`${database}/.sys/top_partitions_one_hour\` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If database is empty (as far as I can see - it can be '' because we are checking its value in ternary above) - will /.sys/... be okay as select source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.sys
will work, /.sys
- won't. However, we alway sent request to specific database, otherwise queries may not work at all (currently I have errors when I sent requests without defined database).
I think it's better to remove ternaries - they have no sense.
Also I found more complex issue, we don't determine db correctly for nested entities, so for nested databases we have wrong db anyway. So actually, there is no difference, whether we pass database or not, .sys
path could be relative when db is passed. Probably I revert the changes where I add database to .sys
path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unneeded ternaries, database should never be empty or request won't be successful anyway. BTW, these ternaries were not needed even in case of empty string database, YQL processed it correctly, they were needed for the case of undefined
database.
Used relative paths everywhere: both in query text and api request we use the same param, so it never differs. If we use relative path, the query will use path from the database, that we passed.
: createShardQueryHistorical( | ||
path, | ||
database, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I see here path and database can exist simulteneously
Could you please explain the real difference (as somewhere we change path to database and somewhere not)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example: we look at shards of a table db1/table1
Database is db1
, it should be used as a request param, system tables in db1/.sys/
, but the result should be filtered, so only shards of current path db1/table1
will be shown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path
- path to schema object
database
- database, where this schema object exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #1490
Why to mark queries?
Also I added database to all queries, so requests will use absolute paths and work in nested databases.Renamedpath
todatabase
in some places for clarityCI Results
Test Status: ✅ PASSED
📊 Full Report
Test Changes Summary ⏭️1
⏭️ Skipped Tests (1)
Bundle Size: ✅
Current: 80.81 MB | Main: 80.81 MB
Diff: 0.01 KB (-0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information