Cannot not extract queryId exactly in code, is that a bug?

[BruceKellan]

Hi, thanks for your repo, it give me a lot of inspiration.
But I found a bug, this is code segment:

presto-gateway/gateway-ha/src/main/java/com/lyft/data/gateway/ha/handler/QueryIdCachingProxyHandler.java

Line 156 in 39d7d5f

protected String extractQueryIdIfPresent(HttpServletRequest request) {

It use a overload method extractQueryIdIfPresent(path, queryParams), and the test case TestQueryIdCachingProxyHandler test extractQueryIdIfPresent(path, queryParams).

But if the request url is "/ui/query.html?20200416_160256_03078_6b4yt", path will be "/ui/query.html" and queryParams will be "20200416_160256_03078_6b4yt", in this case, we cannot not extract queryId exactly and get null

Logic is not the same as test case.

presto-gateway/gateway-ha/src/test/java/com/lyft/data/gateway/ha/handler/TestQueryIdCachingProxyHandler.java

Line 26 in 39d7d5f

String queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);

I think it's a bug, isn't it?

[amitds1997]

Sorry, if I misunderstood your question, why can we not extract the queryId?

/ui/query.html?20200416_160256_03078_6b4yt satisfies the query Id pattern and should be extractable.

presto-gateway/gateway-ha/src/main/java/com/lyft/data/gateway/ha/handler/QueryIdCachingProxyHandler.java

Line 42 in 39d7d5f

private static final Pattern QUERY_ID_PATTERN = Pattern.compile(".*[/=?](\\d+_\\d+_\\d+_\\w+).*");

This is used when a Trino UI path is passed to the gateway.

presto-gateway/gateway-ha/src/main/java/com/lyft/data/gateway/ha/handler/QueryIdCachingProxyHandler.java

Line 204 in 39d7d5f

Matcher matcher = QUERY_ID_PATTERN.matcher(path);

[BruceKellan]

Hi, amitds1997, thanks for your reply.
My question is that the variable path is request.getRequestURI()

presto-gateway/gateway-ha/src/main/java/com/lyft/data/gateway/ha/handler/QueryIdCachingProxyHandler.java

Line 157 in 39d7d5f

String path = request.getRequestURI();

So the variable path will be /ui/query.html without 20200416_160256_03078_6b4yt, the variable queryParams will be 20200416_160256_03078_6b4yt in this case.

So regexp cannot extract queryId because path is /ui/query.html.

The above is my understanding. I reproduce it in my code.

If there is a problem with my understanding, please tell me.

By the way, my prestosql version is 348, /ui/query.html?20200416_160256_03078_6b4yt is my PrestoSQL UI path。

[BruceKellan]

I have looked at other fork repo, such as rongfengliang's presto-gateway,
In his repo, he use the queryParams to be queryId in the same place.

https://github.com/rongfengliang/presto-gateway/blob/7be1c438868b059b375e3af91e40ab2a061988ca/gateway/src/main/java/com/lyft/data/gateway/handler/QueryIdCachingProxyHandler.java#L201

You can have a see.

[amitds1997]

Yes, that seems to be a bug.