Skip to content

Commit

Permalink
Review comments and add ClientSqlParameter for the equivalent SqlPara…
Browse files Browse the repository at this point in the history
…mter.
  • Loading branch information
abhishekrb19 committed Dec 6, 2024
1 parent 0a4747d commit ea7a58d
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.Objects;

/**
* Client representation of {@link org.apache.druid.sql.http.SqlQuery}. This is effectively a thin POJO class for
* Client representation of {@link org.apache.druid.sql.http.SqlQuery}. This is effectively a lightweight POJO class for
* use by clients such as {@link org.apache.druid.client.broker.BrokerClient} that doesn't bring in any of the
* Calcite dependencies and server-side logic from the Broker.
*/
Expand All @@ -52,7 +52,7 @@ public class ClientSqlQuery
private final Map<String, Object> context;

@JsonProperty
private final List<String> parameters;
private final List<ClientSqlParameter> parameters;

@JsonCreator
public ClientSqlQuery(
Expand All @@ -62,7 +62,7 @@ public ClientSqlQuery(
@JsonProperty("typesHeader") final boolean typesHeader,
@JsonProperty("sqlTypesHeader") final boolean sqlTypesHeader,
@JsonProperty("context") final Map<String, Object> context,
@JsonProperty("parameters") final List<String> parameters
@JsonProperty("parameters") final List<ClientSqlParameter> parameters
)
{
this.query = query;
Expand Down Expand Up @@ -104,7 +104,7 @@ public Map<String, Object> getContext()
return context;
}

public List<String> getParameters()
public List<ClientSqlParameter> getParameters()
{
return parameters;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -42,4 +43,20 @@ public void testSerde() throws JsonProcessingException
);
Assert.assertEquals(query, jsonMapper.readValue(jsonMapper.writeValueAsString(query), ClientSqlQuery.class));
}

@Test
public void testSerdeWithParameters() throws JsonProcessingException
{
final ObjectMapper jsonMapper = new DefaultObjectMapper();
final ClientSqlQuery query = new ClientSqlQuery(
"SELECT 1",
"array",
true,
true,
true,
null,
ImmutableList.of(new ClientSqlParameter("ARRAY", ImmutableList.of(-25.7, 20.2, 36.85)))
);
Assert.assertEquals(query, jsonMapper.readValue(jsonMapper.writeValueAsString(query), ClientSqlQuery.class));
}
}
5 changes: 5 additions & 0 deletions sql/src/main/java/org/apache/druid/sql/http/SqlParameter.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
import java.sql.Date;
import java.util.Objects;

/**
* See {@link org.apache.druid.query.http.ClientSqlParameter} for the equivalent POJO class used on the client side.
* Note: The field {@link #type} relies on a Calcite data type, which prevents this class from being moved
* to the processing module for reuse.
*/
public class SqlParameter
{
private final SqlType type;
Expand Down
4 changes: 3 additions & 1 deletion sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
import java.util.stream.Collectors;

/**
* @see ClientSqlQuery for the equivalent POJO class for use on the client side.
* See {@link ClientSqlQuery} for the equivalent POJO class used on the client side to interact with the Broker.
* Note: The fields {@link #resultFormat} and {@link #parameters} rely on Calcite data types,
* preventing this class from being moved to the processing module for reuse.
*/
public class SqlQuery
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@

package org.apache.druid.sql.calcite.http;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.apache.calcite.avatica.SqlType;
import org.apache.druid.query.http.ClientSqlParameter;
import org.apache.druid.query.http.ClientSqlQuery;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.sql.calcite.util.CalciteTestBase;
import org.apache.druid.sql.http.ResultFormat;
Expand All @@ -34,10 +37,11 @@

public class SqlQueryTest extends CalciteTestBase
{
private static final ObjectMapper JSON_MAPPER = TestHelper.makeJsonMapper();

@Test
public void testSerde() throws Exception
{
final ObjectMapper jsonMapper = TestHelper.makeJsonMapper();
final SqlQuery query = new SqlQuery(
"SELECT ?",
ResultFormat.ARRAY,
Expand All @@ -47,7 +51,63 @@ public void testSerde() throws Exception
ImmutableMap.of("useCache", false),
ImmutableList.of(new SqlParameter(SqlType.INTEGER, 1))
);
Assert.assertEquals(query, jsonMapper.readValue(jsonMapper.writeValueAsString(query), SqlQuery.class));
Assert.assertEquals(query, JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(query), SqlQuery.class));
}

@Test
public void testClientSqlQueryToSqlQueryConversion() throws JsonProcessingException
{
final ClientSqlQuery givenClientSqlQuery = new ClientSqlQuery(
"SELECT ?",
"array",
true,
true,
true,
ImmutableMap.of("useCache", false),
null
);

final SqlQuery expectedSqlQuery = new SqlQuery(
"SELECT ?",
ResultFormat.ARRAY,
true,
true,
true,
ImmutableMap.of("useCache", false),
null
);

final SqlQuery observedSqlQuery =
JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(givenClientSqlQuery), SqlQuery.class);
Assert.assertEquals(expectedSqlQuery, observedSqlQuery);
}

@Test
public void testClientSqlQueryToSqlQueryConversion2() throws JsonProcessingException
{
final ClientSqlQuery givenClientSqlQuery = new ClientSqlQuery(
"SELECT ?",
"arrayLines",
false,
false,
false,
ImmutableMap.of("useCache", false),
ImmutableList.of(new ClientSqlParameter("INTEGER", 1), new ClientSqlParameter("VARCHAR", "foo"))
);

final SqlQuery expectedSqlQuery = new SqlQuery(
"SELECT ?",
ResultFormat.ARRAYLINES,
false,
false,
false,
ImmutableMap.of("useCache", false),
ImmutableList.of(new SqlParameter(SqlType.INTEGER, 1), new SqlParameter(SqlType.VARCHAR, "foo"))
);

final SqlQuery observedSqlQuery =
JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(givenClientSqlQuery), SqlQuery.class);
Assert.assertEquals(expectedSqlQuery, observedSqlQuery);
}

@Test
Expand Down

0 comments on commit ea7a58d

Please sign in to comment.