Skip to content

Commit 45e95ab

Browse files
author
Julien Ruaux
committed
fix: Aggregation LIMIT operation should be last
1 parent 0c95e1b commit 45e95ab

14 files changed

+375
-118
lines changed

‎.github/workflows/build.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
- name: Set up Java
1616
uses: actions/setup-java@v1
1717
with:
18-
java-version: 11
18+
java-version: 18
1919

2020
- uses: actions/cache@v2
2121
with:

‎.github/workflows/early-access.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
- name: Set up Java
1919
uses: actions/setup-java@v1
2020
with:
21-
java-version: 17
21+
java-version: 18
2222

2323
- uses: actions/cache@v2
2424
with:

‎.github/workflows/release.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jobs:
1919
- name: Set up Java
2020
uses: actions/setup-java@v1
2121
with:
22-
java-version: 17
22+
java-version: 18
2323

2424
- uses: actions/cache@v2
2525
with:

‎Dockerfile‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
ARG TRINO_VERSION=393
1+
ARG TRINO_VERSION=395
22

3-
FROM docker.io/library/maven:3.6.3-openjdk-11 AS builder
3+
FROM docker.io/library/maven:3.8.6-openjdk-18 AS builder
44
WORKDIR /root/trino-redisearch
55
COPY . /root/trino-redisearch
66
ENV MAVEN_FAST_INSTALL="-DskipTests -Dair.check.skip-all=true -Dmaven.javadoc.skip=true -B -q -T C1"
@@ -11,7 +11,8 @@ FROM trinodb/trino:${TRINO_VERSION}
1111
COPY --from=builder --chown=trino:trino /root/trino-redisearch/target/trino-redisearch-*/* /usr/lib/trino/plugin/redisearch/
1212

1313
USER root:root
14-
RUN yum -y -q install gettext
14+
RUN apt-get update
15+
RUN apt-get install -y -q gettext
1516
COPY --chown=trino:trino docker/etc /etc/trino
1617
COPY docker/template docker/setup.sh /tmp/
1718

‎pom.xml‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,22 @@
55
<parent>
66
<groupId>io.trino</groupId>
77
<artifactId>trino-root</artifactId>
8-
<version>393</version>
8+
<version>395</version>
99
</parent>
1010

1111
<groupId>com.redis</groupId>
1212
<artifactId>trino-redisearch</artifactId>
13+
<version>0.2.4-SNAPSHOT</version>
1314
<description>Trino - RediSearch Connector</description>
1415
<packaging>trino-plugin</packaging>
1516

1617
<properties>
17-
<lettucemod.version>3.0.4</lettucemod.version>
18+
<trino.version>395</trino.version>
19+
<lettucemod.version>3.1.2</lettucemod.version>
1820
<lettuce.version>6.2.0.RELEASE</lettuce.version>
1921
<testcontainers-redis.version>1.6.2</testcontainers-redis.version>
2022
<ulid.version>5.0.0</ulid.version>
21-
<air.check.skip-enforcer>true</air.check.skip-enforcer>
23+
2224
<air.check.skip-license>true</air.check.skip-license>
2325
<air.check.skip-checkstyle>true</air.check.skip-checkstyle>
2426
</properties>
@@ -28,7 +30,7 @@
2830
<dependency>
2931
<groupId>io.trino</groupId>
3032
<artifactId>trino-plugin-toolkit</artifactId>
31-
<scope>compile</scope>
33+
<version>${trino.version}</version>
3234
</dependency>
3335

3436
<dependency>
@@ -94,6 +96,13 @@
9496
<artifactId>validation-api</artifactId>
9597
</dependency>
9698

99+
<dependency>
100+
<groupId>io.trino</groupId>
101+
<artifactId>trino-matching</artifactId>
102+
<version>${trino.version}</version>
103+
<scope>runtime</scope>
104+
</dependency>
105+
97106
<!-- used by tests but also needed transitively -->
98107
<dependency>
99108
<groupId>io.airlift</groupId>
@@ -105,6 +114,7 @@
105114
<dependency>
106115
<groupId>io.trino</groupId>
107116
<artifactId>trino-spi</artifactId>
117+
<version>${trino.version}</version>
108118
<scope>provided</scope>
109119
</dependency>
110120

@@ -120,6 +130,7 @@
120130
<scope>provided</scope>
121131
</dependency>
122132

133+
123134
<dependency>
124135
<groupId>org.openjdk.jol</groupId>
125136
<artifactId>jol-core</artifactId>
@@ -131,36 +142,42 @@
131142
<dependency>
132143
<groupId>io.trino</groupId>
133144
<artifactId>trino-array</artifactId>
145+
<version>${trino.version}</version>
134146
<scope>test</scope>
135147
</dependency>
136148

137149
<dependency>
138150
<groupId>io.trino</groupId>
139151
<artifactId>trino-client</artifactId>
152+
<version>${trino.version}</version>
140153
<scope>test</scope>
141154
</dependency>
142155

143156
<dependency>
144157
<groupId>io.trino</groupId>
145158
<artifactId>trino-collect</artifactId>
159+
<version>${trino.version}</version>
146160
<scope>test</scope>
147161
</dependency>
148162

149163
<dependency>
150164
<groupId>io.trino</groupId>
151165
<artifactId>trino-geospatial-toolkit</artifactId>
166+
<version>${trino.version}</version>
152167
<scope>test</scope>
153168
</dependency>
154169

155170
<dependency>
156171
<groupId>io.trino</groupId>
157172
<artifactId>trino-jmx</artifactId>
173+
<version>${trino.version}</version>
158174
<scope>test</scope>
159175
</dependency>
160176

161177
<dependency>
162178
<groupId>io.trino</groupId>
163179
<artifactId>trino-main</artifactId>
180+
<version>${trino.version}</version>
164181
<scope>test</scope>
165182

166183
<exclusions>
@@ -175,6 +192,7 @@
175192
<groupId>io.trino</groupId>
176193
<artifactId>trino-main</artifactId>
177194
<type>test-jar</type>
195+
<version>${trino.version}</version>
178196
<scope>test</scope>
179197

180198
<exclusions>
@@ -188,30 +206,35 @@
188206
<dependency>
189207
<groupId>io.trino</groupId>
190208
<artifactId>trino-memory-context</artifactId>
209+
<version>${trino.version}</version>
191210
<scope>test</scope>
192211
</dependency>
193212

194213
<dependency>
195214
<groupId>io.trino</groupId>
196215
<artifactId>trino-parser</artifactId>
216+
<version>${trino.version}</version>
197217
<scope>test</scope>
198218
</dependency>
199219

200220
<dependency>
201221
<groupId>io.trino</groupId>
202222
<artifactId>trino-testing</artifactId>
223+
<version>${trino.version}</version>
203224
<scope>test</scope>
204225
</dependency>
205226

206227
<dependency>
207228
<groupId>io.trino</groupId>
208229
<artifactId>trino-testing-services</artifactId>
230+
<version>${trino.version}</version>
209231
<scope>test</scope>
210232
</dependency>
211233

212234
<dependency>
213235
<groupId>io.trino</groupId>
214236
<artifactId>trino-tpch</artifactId>
237+
<version>${trino.version}</version>
215238
<scope>test</scope>
216239
</dependency>
217240

@@ -331,7 +354,6 @@
331354
<scope>test</scope>
332355
</dependency>
333356

334-
335357
<dependency>
336358
<groupId>org.testcontainers</groupId>
337359
<artifactId>testcontainers</artifactId>

‎src/main/java/com/redis/trino/RediSearchClientModule.java‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@
2626
import static io.airlift.configuration.ConfigBinder.configBinder;
2727
import static java.util.Objects.requireNonNull;
2828

29-
import java.time.Duration;
30-
3129
import javax.inject.Singleton;
3230

3331
import com.google.inject.Binder;
3432
import com.google.inject.Module;
3533
import com.google.inject.Provides;
3634
import com.google.inject.Scopes;
37-
import com.redis.lettucemod.RedisModulesClient;
35+
import com.redis.lettucemod.util.RedisClientBuilder;
36+
import com.redis.lettucemod.util.RedisClientOptions;
37+
import com.redis.lettucemod.util.RedisClientOptions.Builder;
3838

39-
import io.lettuce.core.RedisURI;
39+
import io.lettuce.core.SslVerifyMode;
4040
import io.trino.spi.type.TypeManager;
4141

4242
public class RediSearchClientModule implements Module {
@@ -51,28 +51,23 @@ public void configure(Binder binder) {
5151
configBinder(binder).bindConfig(RediSearchConfig.class);
5252
}
5353

54-
@SuppressWarnings("deprecation")
5554
@Singleton
5655
@Provides
5756
public static RediSearchSession createRediSearchSession(TypeManager typeManager, RediSearchConfig config) {
5857
requireNonNull(config, "config is null");
59-
if (config.getUri().isPresent()) {
60-
RedisURI uri = RedisURI.create(config.getUri().get());
61-
if (config.isInsecure()) {
62-
uri.setVerifyPeer(false);
63-
}
64-
if (config.isTls()) {
65-
uri.setSsl(true);
66-
}
67-
config.getUsername().ifPresent(uri::setUsername);
68-
config.getPassword().ifPresent(uri::setPassword);
69-
if (config.getTimeout() > 0) {
70-
uri.setTimeout(Duration.ofSeconds(config.getTimeout()));
71-
}
72-
RedisModulesClient client = RedisModulesClient.create(uri);
73-
return new RediSearchSession(typeManager, client.connect(), config);
58+
Builder options = RedisClientOptions.builder();
59+
options.uriString(config.getUri());
60+
options.ssl(config.isTls());
61+
options.username(config.getUsername());
62+
options.password(config.getPassword());
63+
if (config.isInsecure()) {
64+
options.sslVerifyMode(SslVerifyMode.NONE);
65+
}
66+
if (config.getTimeout() > 0) {
67+
options.timeoutInSeconds(config.getTimeout());
7468
}
75-
throw new IllegalArgumentException("No Redis URI specified");
69+
RedisClientBuilder clientBuilder = RedisClientBuilder.create(options.build());
70+
return new RediSearchSession(typeManager, clientBuilder.client(), config);
7671
}
7772

7873
}

‎src/main/java/com/redis/trino/RediSearchConfig.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.time.Duration;
2727
import java.util.Optional;
2828

29+
import javax.validation.constraints.Min;
2930
import javax.validation.constraints.NotNull;
3031
import javax.validation.constraints.Pattern;
3132

@@ -44,15 +45,16 @@ public class RediSearchConfig {
4445
private Optional<String> uri = Optional.empty();
4546
private Optional<String> username = Optional.empty();
4647
private Optional<String> password = Optional.empty();
47-
private boolean caseInsensitiveNameMatching;
4848
private boolean insecure;
4949
private boolean tls;
50-
private long timeout = 0; // Use Lettuce default
50+
private long timeout; // Use Lettuce default
51+
private boolean caseInsensitiveNameMatching;
5152
private long defaultLimit = DEFAULT_LIMIT;
52-
private long cursorCount = 0; // Use RediSearch default
53+
private long cursorCount; // Use RediSearch default
5354
private long tableCacheExpiration = DEFAULT_TABLE_CACHE_EXPIRATION.toSeconds();
5455
private long tableCacheRefresh = DEFAULT_TABLE_CACHE_REFRESH.toSeconds();
5556

57+
@Min(0)
5658
public long getCursorCount() {
5759
return cursorCount;
5860
}
@@ -178,6 +180,7 @@ public RediSearchConfig setTls(boolean tls) {
178180
return this;
179181
}
180182

183+
@Min(0)
181184
public long getTimeout() {
182185
return timeout;
183186
}
@@ -188,4 +191,5 @@ public RediSearchConfig setTimeout(long timeout) {
188191
this.timeout = timeout;
189192
return this;
190193
}
194+
191195
}

‎src/main/java/com/redis/trino/RediSearchConnector.java‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,25 @@
2323
*/
2424
package com.redis.trino;
2525

26+
import static com.google.common.base.Preconditions.checkArgument;
27+
import static io.trino.spi.transaction.IsolationLevel.READ_UNCOMMITTED;
28+
import static io.trino.spi.transaction.IsolationLevel.checkConnectorSupports;
29+
import static java.util.Objects.requireNonNull;
30+
31+
import java.util.concurrent.ConcurrentHashMap;
32+
import java.util.concurrent.ConcurrentMap;
33+
34+
import javax.inject.Inject;
35+
2636
import io.trino.spi.connector.Connector;
2737
import io.trino.spi.connector.ConnectorMetadata;
2838
import io.trino.spi.connector.ConnectorPageSinkProvider;
2939
import io.trino.spi.connector.ConnectorPageSourceProvider;
40+
import io.trino.spi.connector.ConnectorSession;
3041
import io.trino.spi.connector.ConnectorSplitManager;
3142
import io.trino.spi.connector.ConnectorTransactionHandle;
3243
import io.trino.spi.transaction.IsolationLevel;
3344

34-
import javax.inject.Inject;
35-
36-
import java.util.concurrent.ConcurrentHashMap;
37-
import java.util.concurrent.ConcurrentMap;
38-
39-
import static com.google.common.base.Preconditions.checkArgument;
40-
import static io.trino.spi.transaction.IsolationLevel.READ_UNCOMMITTED;
41-
import static io.trino.spi.transaction.IsolationLevel.checkConnectorSupports;
42-
import static java.util.Objects.requireNonNull;
43-
4445
public class RediSearchConnector implements Connector {
4546

4647
private final RediSearchSession rediSearchSession;
@@ -67,30 +68,23 @@ public ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLevel
6768
transactions.put(transaction, new RediSearchMetadata(rediSearchSession));
6869
return transaction;
6970
}
70-
71+
7172
@Override
72-
public ConnectorMetadata getMetadata(ConnectorTransactionHandle transaction) {
73-
RediSearchMetadata metadata = transactions.get(transaction);
74-
checkTransaction(metadata, transaction);
73+
public ConnectorMetadata getMetadata(ConnectorSession session, ConnectorTransactionHandle transactionHandle) {
74+
RediSearchMetadata metadata = transactions.get(transactionHandle);
75+
checkTransaction(metadata, transactionHandle);
7576
return metadata;
7677
}
7778

78-
private void checkTransaction(Object object, ConnectorTransactionHandle transaction) {
79-
checkArgument(object != null, "no such transaction: %s", transaction);
79+
private void checkTransaction(Object object, ConnectorTransactionHandle transactionHandle) {
80+
checkArgument(object != null, "no such transaction: %s", transactionHandle);
8081
}
8182

8283
@Override
8384
public void commit(ConnectorTransactionHandle transaction) {
8485
checkTransaction(transactions.remove(transaction), transaction);
8586
}
8687

87-
// @Override
88-
// public void rollback(ConnectorTransactionHandle transaction) {
89-
// RediSearchMetadata metadata = transactions.remove(transaction);
90-
// checkTransaction(metadata, transaction);
91-
// metadata.rollback();
92-
// }
93-
9488
@Override
9589
public ConnectorSplitManager getSplitManager() {
9690
return splitManager;

‎src/main/java/com/redis/trino/RediSearchMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ public Optional<AggregationApplicationResult<ConnectorTableHandle>> applyAggrega
312312
RediSearchTableHandle tableHandle = new RediSearchTableHandle(Type.AGGREGATE, table.getSchemaTableName(),
313313
table.getConstraint(), table.getLimit(), termAggregations.build(), metrics);
314314
return Optional.of(new AggregationApplicationResult<>(tableHandle, projections.build(),
315-
resultAssignments.build(), ImmutableMap.of(), false));
315+
resultAssignments.build(), Map.of(), false));
316316
}
317317

318318
private void setRollback(Runnable action) {

0 commit comments

Comments
 (0)