From 40d8d9f27304fd87ca55e20199762558423367d5 Mon Sep 17 00:00:00 2001 From: vamsi-amazon Date: Tue, 1 Nov 2022 10:25:28 -0700 Subject: [PATCH] Describe Table with catalog name. (#989) Signed-off-by: Vamsi Manohar Signed-off-by: Vamsi Manohar --- docs/user/general/identifiers.rst | 86 +++++++++++++++++++ docs/user/ppl/cmd/describe.rst | 28 +++++- .../opensearch/sql/ppl/DescribeCommandIT.java | 23 +++++ .../opensearch/sql/ppl/parser/AstBuilder.java | 12 ++- .../sql/ppl/parser/AstBuilderTest.java | 8 ++ .../client/PrometheusClientImpl.java | 7 +- .../client/PrometheusClientImplTest.java | 1 - 7 files changed, 159 insertions(+), 6 deletions(-) diff --git a/docs/user/general/identifiers.rst b/docs/user/general/identifiers.rst index b211930884..affd381d41 100644 --- a/docs/user/general/identifiers.rst +++ b/docs/user/general/identifiers.rst @@ -176,3 +176,89 @@ Query delimited multiple indices seperated by ``,``:: |-------| | 5 | +-------+ + + + +Fully Qualified Table Names +=========================== + +Description +----------- +With the introduction of different datasource catalogs along with Opensearch, support for fully qualified table names became compulsory to resolve tables to a catalog. + +Format for fully qualified table name. +``..`` + +* catalogName:[Mandatory] Catalog information is mandatory when querying over tables from catalogs other than opensearch connector. + +* schemaName:[Optional] Schema is a logical abstraction for a group of tables. In the current state, we only support ``default`` and ``information_schema``. Any schema mentioned in the fully qualified name other than these two will be resolved to be part of tableName. + +* tableName:[Mandatory] tableName is mandatory. + +The current resolution algorithm works in such a way, the old queries on opensearch work without specifying any catalog name. +So queries on opensearch indices doesn't need a fully qualified table name. + +Table Name Resolution Algorithm. +-------------------------------- + +Fully qualified Name is divided into parts based on ``.`` character. + +TableName resolution algorithm works in the following manner. + +1. Take the first part of the qualified name and resolve it to a catalog from the list of catalogs configured. +If it doesn't resolve to any of the catalog names configured, catalog name will default to ``@opensearch`` catalog. + +2. Take the first part of the remaining qualified name after capturing the catalog name. +If this part represents any of the supported schemas under catalog, it will resolve to the same otherwise schema name will resolve to ``default`` schema. +Currently ``default`` and ``information_schema`` are the only schemas supported. + +3. Rest of the parts are combined to resolve tablename. + +** Only table name identifiers are supported with fully qualified names, identifiers used for columns and other attributes doesn't require prefixing with catalog and schema information.** + +Examples +-------- +Assume [my_prometheus] is the only catalog configured other than default opensearch engine. + +1. ``my_prometheus.default.http_requests_total`` + +catalogName = ``my_prometheus`` [Is in the list of catalogs configured]. + +schemaName = ``default`` [Is in the list of schemas supported]. + +tableName = ``http_requests_total``. + +2. ``logs.12.13.1`` + + +catalogName = ``@opensearch`` [Resolves to default @opensearch connector since [my_prometheus] is the only catalog configured name.] + +schemaName = ``default`` [No supported schema found, so default to `default`]. + +tableName = ``logs.12.13.1``. + + +3. ``my_prometheus.http_requests_total`` + + +catalogName = ```my_prometheus`` [Is in the list of catalogs configured]. + +schemaName = ``default`` [No supported schema found, so default to `default`]. + +tableName = ``http_requests_total``. + +4. ``prometheus.http_requests_total`` + +catalogName = ``@opensearch`` [Resolves to default @opensearch connector since [my_prometheus] is the only catalog configured name.] + +schemaName = ``default`` [No supported schema found, so default to `default`]. + +tableName = ``prometheus.http_requests_total``. + +5. ``prometheus.default.http_requests_total.1.2.3`` + +catalogName = ``@opensearch`` [Resolves to default @opensearch connector since [my_prometheus] is the only catalog configured name.] + +schemaName = ``default`` [No supported schema found, so default to `default`]. + +tableName = ``prometheus.default.http_requests_total.1.2.3``. diff --git a/docs/user/ppl/cmd/describe.rst b/docs/user/ppl/cmd/describe.rst index 0abd569684..c683aa1781 100644 --- a/docs/user/ppl/cmd/describe.rst +++ b/docs/user/ppl/cmd/describe.rst @@ -16,9 +16,12 @@ Description Syntax ============ -describe +describe .. + +* catalog: optional. If catalog is not provided, it resolves to opensearch catalog. +* schema: optional. If schema is not provided, it resolves to default schema. +* tablename: mandatory. describe command must specify which tablename to query from. -* index: mandatory. describe command must specify which index to query from. Example 1: Fetch all the metadata @@ -63,3 +66,24 @@ PPL query:: | age | +----------------+ + +Example 3: Fetch metadata for table in prometheus catalog +========================================================= + +The example retrieves table info for ``prometheus_http_requests_total`` metric in prometheus catalog. + +PPL query:: + + os> describe my_prometheus.prometheus_http_requests_total; + fetched rows / total rows = 7/7 + +-----------------+----------------+--------------------------------+---------------+-------------+ + | TABLE_CATALOG | TABLE_SCHEMA | TABLE_NAME | COLUMN_NAME | DATA_TYPE | + |-----------------+----------------+--------------------------------+---------------+-------------| + | my_prometheus | default | prometheus_http_requests_total | @labels | keyword | + | my_prometheus | default | prometheus_http_requests_total | handler | keyword | + | my_prometheus | default | prometheus_http_requests_total | code | keyword | + | my_prometheus | default | prometheus_http_requests_total | instance | keyword | + | my_prometheus | default | prometheus_http_requests_total | @timestamp | timestamp | + | my_prometheus | default | prometheus_http_requests_total | @value | double | + | my_prometheus | default | prometheus_http_requests_total | job | keyword | + +-----------------+----------------+--------------------------------+---------------+-------------+ diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java index c06ef3bc21..88ef028fbe 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java @@ -15,6 +15,7 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; import static org.opensearch.sql.util.MatcherUtils.columnName; +import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.verifyColumn; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; @@ -87,4 +88,26 @@ public void describeCommandWithoutIndexShouldFailToParse() throws IOException { assertTrue(e.getMessage().contains("Failed to parse query due to offending symbol")); } } + + @Test + public void testDescribeCommandWithPrometheusCatalog() throws IOException { + JSONObject result = executeQuery("describe my_prometheus.prometheus_http_requests_total"); + verifyColumn( + result, + columnName("TABLE_CATALOG"), + columnName("TABLE_SCHEMA"), + columnName("TABLE_NAME"), + columnName("COLUMN_NAME"), + columnName("DATA_TYPE") + ); + verifyDataRows(result, + rows("my_prometheus", "default", "prometheus_http_requests_total", "@labels", "keyword"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "handler", "keyword"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "code", "keyword"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "instance", "keyword"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "@value", "double"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "@timestamp", + "timestamp"), + rows("my_prometheus", "default", "prometheus_http_requests_total", "job", "keyword")); + } } diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index 13d8f8cddf..699d671bb2 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -44,6 +45,7 @@ import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Map; import org.opensearch.sql.ast.expression.ParseMethod; +import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.UnresolvedArgument; import org.opensearch.sql.ast.expression.UnresolvedExpression; import org.opensearch.sql.ast.tree.AD; @@ -118,11 +120,19 @@ public UnresolvedPlan visitSearchFilterFrom(SearchFilterFromContext ctx) { /** * Describe command. + * Current logic separates table and metadata info about table by adding + * MAPPING_ODFE_SYS_TABLE as suffix. + * Even with the introduction of catalog and schema name in fully qualified table name, + * we do the same thing by appending MAPPING_ODFE_SYS_TABLE as syffix to the last part + * of qualified name. */ @Override public UnresolvedPlan visitDescribeCommand(DescribeCommandContext ctx) { final Relation table = (Relation) visitTableSourceClause(ctx.tableSourceClause()); - return new Relation(qualifiedName(mappingTable(table.getTableName()))); + QualifiedName tableQualifiedName = table.getTableQualifiedName(); + ArrayList parts = new ArrayList<>(tableQualifiedName.getParts()); + parts.set(parts.size() - 1, mappingTable(parts.get(parts.size() - 1))); + return new Relation(new QualifiedName(parts)); } /** diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index eb2f651ac7..26e44b674d 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -739,6 +739,14 @@ public void testDescribeCommandWithMultipleIndices() { relation(mappingTable("t,u"))); } + @Test + public void testDescribeCommandWithFullyQualifiedTableName() { + assertEqual("describe prometheus.http_metric", + relation(qualifiedName("prometheus", mappingTable("http_metric")))); + assertEqual("describe prometheus.schema.http_metric", + relation(qualifiedName("prometheus", "schema", mappingTable("http_metric")))); + } + @Test public void test_fitRCFADCommand_withoutDataFormat() { assertEqual("source=t | AD shingle_size=10 time_decay=0.0001 time_field='timestamp' " diff --git a/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java b/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java index cd9aed4d18..1c427cd3a7 100644 --- a/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java +++ b/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java @@ -14,7 +14,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; @@ -82,7 +81,11 @@ public Map> getAllMetrics() throws IOException { private List toListOfStrings(JSONArray array) { List result = new ArrayList<>(); for (int i = 0; i < array.length(); i++) { - result.add(array.optString(i)); + //__name__ is internal label in prometheus representing the metric name. + //Exempting this from labels list as it is not required in any of the operations. + if (!"__name__".equals(array.optString(i))) { + result.add(array.optString(i)); + } } return result; } diff --git a/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java b/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java index 5f4715fca7..f49672a882 100644 --- a/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java +++ b/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java @@ -106,7 +106,6 @@ void testGetLabel() { mockWebServer.enqueue(mockResponse); List response = prometheusClient.getLabels(METRIC_NAME); assertEquals(new ArrayList() {{ - add("__name__"); add("call"); add("code"); }