Skip to content
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

Adds a built-in PartiQL System catalog, support for SQL-Path, and SPI cleanup #1670

Merged
merged 13 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions partiql-spi/api/partiql-spi.api
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ public final class org/partiql/spi/catalog/Path$Companion {

public abstract interface class org/partiql/spi/catalog/Session {
public static final field Companion Lorg/partiql/spi/catalog/Session$Companion;
public static fun bare (Ljava/lang/String;)Lorg/partiql/spi/catalog/Session;
public static fun builder ()Lorg/partiql/spi/catalog/Session$Builder;
public static fun empty ()Lorg/partiql/spi/catalog/Session;
public abstract fun getCatalog ()Ljava/lang/String;
Expand All @@ -261,9 +262,11 @@ public final class org/partiql/spi/catalog/Session$Builder {
public final fun namespace (Lorg/partiql/spi/catalog/Namespace;)Lorg/partiql/spi/catalog/Session$Builder;
public final fun namespace ([Ljava/lang/String;)Lorg/partiql/spi/catalog/Session$Builder;
public final fun property (Ljava/lang/String;Ljava/lang/String;)Lorg/partiql/spi/catalog/Session$Builder;
public final fun systemCatalog (Lorg/partiql/spi/catalog/Catalog;)Lorg/partiql/spi/catalog/Session$Builder;
}

public final class org/partiql/spi/catalog/Session$Companion {
public final fun bare (Ljava/lang/String;)Lorg/partiql/spi/catalog/Session;
public final fun builder ()Lorg/partiql/spi/catalog/Session$Builder;
public final fun empty ()Lorg/partiql/spi/catalog/Session;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package org.partiql.spi.catalog;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.partiql.spi.function.Aggregation;
import org.partiql.spi.function.Builtins;
import org.partiql.spi.function.Function;

import java.util.Collection;

/**
* <p>
* This implements the PartiQL System Catalog.
* </p>
* <p>
* It provides the implementation for the PartiQL System Catalog, which is a built-in catalog
* that provides access to the PartiQL language and its built-in functions and aggregations.
* </p>
* @see Session.Builder
*/
final class PartiQLSystemCatalog implements Catalog {

/**
* TODO
*/
@NotNull
private final String name;

/**
* Creates a new PartiQL System Catalog with the given name.
* @param name the name of the PartiQL System Catalog
*/
PartiQLSystemCatalog(@NotNull String name) {
this.name = name;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make singleton with private constructor and have a public static final String for "$system"

So it's like

protected final class System implements Catalog { 

     static final INSTANCE

     static final NAME = "\$system"
}

like that. Also I know that protected is different than package-private, but do prefer an explicit modifier

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't make a protected top-level class. Updated the Javadocs to make the visibility more apparent, since there is no package-private modifier. And added the singleton. See latest commit: d1b3305

If we want to expose this class in the future, sure. But we don't need to right now.


@NotNull
@Override
public String getName() {
return this.name;
}

@Nullable
@Override
public Table getTable(@NotNull Session session, @NotNull Name name) {
return null;
}

@Nullable
@Override
public Table getTable(@NotNull Session session, @NotNull Identifier identifier) {
return null;
}

@NotNull
@Override
public Collection<Function> getFunctions(@NotNull Session session, @NotNull String name) {
return Builtins.INSTANCE.getFunctions(name);
}

@NotNull
@Override
public Collection<Aggregation> getAggregations(@NotNull Session session, @NotNull String name) {
return Builtins.INSTANCE.getAggregations(name);
}
}
38 changes: 36 additions & 2 deletions partiql-spi/src/main/kotlin/org/partiql/spi/catalog/Session.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public interface Session {
public companion object {

/**
* Returns a [Session] with only the "empty" catalog implementation.
* Returns a [Session] with only the "empty" catalog implementation. Note that this does NOT add the
* PartiQL System Catalog.
*/
@JvmStatic
public fun empty(): Session = object : Session {
Copy link
Member

@RCHowell RCHowell Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session.empty() is used in several places so would need the psystem available. I don't believe we need bare() factory method either. I was referring to a builder method in the previous PR, not a factory.

Think about it this way,

Session.builder()....build()   // standard builder comes with standard system
Session.system(catalog)...builder()  // .system() is a named *builder* factory which replaces the standard system.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the latest revision has this. I can just revert the last commit (the addition of the bare method).

Copy link
Member Author

@johnedquinn johnedquinn Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reverted and added the PartiQL system catalog to Session.empty(). See the last three commits. LMK what you think.

Expand All @@ -53,6 +54,19 @@ public interface Session {
override fun getNamespace(): Namespace = Namespace.empty()
}

/**
* Returns a [Session] with only the PartiQL System Catalog. Therefore, this provides PartiQL's built-in functions
* and aggregations.
* @param systemCatalogName the name to assign the system catalog
*/
@JvmStatic
public fun bare(systemCatalogName: String): Session = object : Session {
override fun getIdentity(): String = "unknown"
override fun getCatalog(): String = systemCatalogName
override fun getCatalogs(): Catalogs = Catalogs.builder().add(PartiQLSystemCatalog(systemCatalogName)).build()
override fun getNamespace(): Namespace = Namespace.empty()
}

@JvmStatic
public fun builder(): Builder = Builder()
}
Expand All @@ -64,6 +78,8 @@ public interface Session {

private var identity: String = "unknown"
private var catalog: String? = null
private var systemCatalogName: String = "\$pql_system"
private var systemCatalog: Catalog = PartiQLSystemCatalog(systemCatalogName)
johnedquinn marked this conversation as resolved.
Show resolved Hide resolved
private var catalogs: Catalogs.Builder = Catalogs.builder()
private var namespace: Namespace = Namespace.empty()
private var properties: MutableMap<String, String> = mutableMapOf()
Expand Down Expand Up @@ -98,6 +114,16 @@ public interface Session {
return this
}

/**
* Adds and designates a catalog to always be on the SQL-Path. This [catalog] provides all built-in functions
* to the system at hand.
* If this is never invoked, a default system catalog is provided.
*/
public fun systemCatalog(catalog: Catalog): Builder {
this.systemCatalogName = catalog.getName()
return this
}

/**
* Adds catalogs to this session.
*/
Expand All @@ -110,16 +136,24 @@ public interface Session {

public fun build(): Session = object : Session {

private val _catalogs = catalogs.build()
private val _catalogs: Catalogs
private val systemCatalogNamespace: Namespace = Namespace.of(systemCatalogName)

init {
require(catalog != null) { "Session catalog must be set" }
catalogs.add(systemCatalog)
_catalogs = catalogs.build()
}

override fun getIdentity(): String = identity
override fun getCatalog(): String = catalog!!
override fun getCatalogs(): Catalogs = _catalogs
override fun getNamespace(): Namespace = namespace

override fun getPath(): Path {
val currentNamespace = getNamespace()
return Path.of(currentNamespace, systemCatalogNamespace)
}
}
}
}
Loading