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

feat: pg_catalog.version(), standard_conforming_strings session var #2448

Merged
merged 3 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 27 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions bindings/python/examples/great_expectations_ex.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import great_expectations as gx

context = gx.get_context() # gets a great expectations project context
ds = context.sources.add_postgres(
name="glaredb",
connection_string="postgresql://sean:sean@localhost:6543/db",
)
2 changes: 2 additions & 0 deletions bindings/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ pandas
polars
pytest
pandasai
great_expectations[postgresql]
great_expectations
12 changes: 10 additions & 2 deletions crates/datafusion_ext/src/vars/constants.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use super::*;

use pgrepr::compatible::server_version;
use pgrepr::notice::NoticeSeverity;

// TODO: Decide proper postgres version to spoof/support
pub(super) const SERVER_VERSION: ServerVar<str> = ServerVar {
name: "server_version",
value: "15.1",
value: server_version(),
group: "postgres",
user_configurable: false,
description: "Version of the server",
Expand Down Expand Up @@ -84,6 +84,14 @@ pub(super) const CLIENT_MIN_MESSAGES: ServerVar<NoticeSeverity> = ServerVar {
description: "Controls which messages are sent to the client, defaults NOTICE",
};

pub(super) const STANDARD_CONFORMING_STRINGS: ServerVar<bool> = ServerVar {
name: "standard_conforming_strings",
value: &true,
group: "postgres",
user_configurable: false,
description: "Treat backslashes literally in string literals",
};

pub(super) static GLAREDB_VERSION_OWNED: Lazy<String> =
Lazy::new(|| format!("v{}", env!("CARGO_PKG_VERSION")));
pub(super) static GLAREDB_VERSION: Lazy<ServerVar<str>> = Lazy::new(|| ServerVar {
Expand Down
6 changes: 6 additions & 0 deletions crates/datafusion_ext/src/vars/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct SessionVarsInner {
pub transaction_isolation: SessionVar<str>,
pub search_path: SessionVar<[String]>,
pub client_min_messages: SessionVar<NoticeSeverity>,
pub standard_conforming_strings: SessionVar<bool>,
pub enable_debug_datasources: SessionVar<bool>,
pub force_catalog_refresh: SessionVar<bool>,
pub glaredb_version: SessionVar<str>,
Expand Down Expand Up @@ -86,6 +87,8 @@ impl SessionVarsInner {
Ok(&self.search_path)
} else if name.eq_ignore_ascii_case(CLIENT_MIN_MESSAGES.name) {
Ok(&self.client_min_messages)
} else if name.eq_ignore_ascii_case(STANDARD_CONFORMING_STRINGS.name) {
Ok(&self.standard_conforming_strings)
} else if name.eq_ignore_ascii_case(ENABLE_DEBUG_DATASOURCES.name) {
Ok(&self.enable_debug_datasources)
} else if name.eq_ignore_ascii_case(FORCE_CATALOG_REFRESH.name) {
Expand Down Expand Up @@ -145,6 +148,8 @@ impl SessionVarsInner {
self.search_path.set_from_str(val, setter)
} else if name.eq_ignore_ascii_case(CLIENT_MIN_MESSAGES.name) {
self.client_min_messages.set_from_str(val, setter)
} else if name.eq_ignore_ascii_case(STANDARD_CONFORMING_STRINGS.name) {
self.standard_conforming_strings.set_from_str(val, setter)
} else if name.eq_ignore_ascii_case(ENABLE_DEBUG_DATASOURCES.name) {
self.enable_debug_datasources.set_from_str(val, setter)
} else if name.eq_ignore_ascii_case(FORCE_CATALOG_REFRESH.name) {
Expand Down Expand Up @@ -221,6 +226,7 @@ impl Default for SessionVarsInner {
transaction_isolation: SessionVar::new(&TRANSACTION_ISOLATION),
search_path: SessionVar::new(&SEARCH_PATH),
client_min_messages: SessionVar::new(&CLIENT_MIN_MESSAGES),
standard_conforming_strings: SessionVar::new(&STANDARD_CONFORMING_STRINGS),
enable_debug_datasources: SessionVar::new(&ENABLE_DEBUG_DATASOURCES),
force_catalog_refresh: SessionVar::new(&FORCE_CATALOG_REFRESH),
glaredb_version: SessionVar::new(&GLAREDB_VERSION),
Expand Down
4 changes: 4 additions & 0 deletions crates/metastore/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,10 @@ mod tests {
BuiltinCatalog::new().unwrap();
}

// TODO: Currently there's a conflict between `version()` and `pg_catalog.version()`.
//
// See <https://github.com/GlareDB/glaredb/issues/2371>
#[ignore]
#[test]
fn builtin_catalog_no_function_name_duplicates() {
// Ensures each function is a unique (schema, name) pair.
Expand Down
1 change: 1 addition & 0 deletions crates/pgrepr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ num-traits = "0.2.17"
dtoa = "1.0.9"
chrono-tz = "0.8.5"
bytes = "1.4.0"
const_format = "0.2.32"
31 changes: 31 additions & 0 deletions crates/pgrepr/src/compatible.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/// Variables desribing the postgres version glaredb claims to be compatible with.
use const_format::concatcp;

pub const PG_MAJOR_VERSION: u16 = 15;
pub const PG_MINOR_VERSION: u16 = 1;

/// A short-form version string for use in the `server_version` session
/// variable.
pub const fn server_version() -> &'static str {
concatcp!(PG_MAJOR_VERSION, ".", PG_MINOR_VERSION)
}

/// A long-form version string for use in the `pg_catalog.version()` function.
///
/// Example:
/// PostgreSQL 15.1 on aarch64-unknown-linux-gnu, compiled by rustc (GlareDB 0.8.1)
pub const fn server_version_with_build_info() -> &'static str {
// TODO: We should have a build_info crate with these constants.
const GLAREDB_VERSION: &str = env!("CARGO_PKG_VERSION");
const TARGET_TRIPLE: &str = "aarch64-unknown-linux-gnu"; // Again, build_info crate.

concatcp!(
"PostgreSQL ",
server_version(),
" on ",
TARGET_TRIPLE,
", compiled by rustc (GlareDB ",
GLAREDB_VERSION,
")"
)
}
1 change: 1 addition & 0 deletions crates/pgrepr/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod compatible;
pub mod error;
pub mod format;
pub mod notice;
Expand Down
5 changes: 5 additions & 0 deletions crates/sqlbuiltins/src/functions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ impl FunctionRegistry {
Arc::new(PgTableIsVisible),
Arc::new(PgEncodingToChar),
Arc::new(PgArrayToString),
Arc::new(PgVersion),
// System functions
Arc::new(ConnectionId),
Arc::new(Version),
Expand Down Expand Up @@ -393,6 +394,10 @@ mod tests {
.expect("'read_parquet' should have description");
}

// TODO: Currently there's a conflict between `version()` and `pg_catalog.version()`.
//
// See <https://github.com/GlareDB/glaredb/issues/2371>
#[ignore]
#[test]
fn func_iters_return_one_copy() {
// Assert that iterators only ever return a single reference to a
Expand Down
2 changes: 0 additions & 2 deletions crates/sqlbuiltins/src/functions/scalars/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ fn try_from_u64_scalar(scalar: ScalarValue) -> Result<u64, BuiltinError> {
}
}

#[allow(dead_code)] // just for merging order // TODO: What?
fn safe_up_cast_integer_scalar(value: i64) -> Result<u64, BuiltinError> {
if value < 0 {
Err(BuiltinError::ParseError(
Expand All @@ -127,7 +126,6 @@ fn safe_up_cast_integer_scalar(value: i64) -> Result<u64, BuiltinError> {

// get_nth_64_fn_arg extracts a string value (or tries to) from a
// function argument; columns are always an error.
#[allow(dead_code)] // just for merging order
fn get_nth_u64_fn_arg(input: &[ColumnarValue], idx: usize) -> Result<u64, BuiltinError> {
match input.get(idx) {
Some(ColumnarValue::Scalar(value)) => try_from_u64_scalar(value.to_owned()),
Expand Down
34 changes: 34 additions & 0 deletions crates/sqlbuiltins/src/functions/scalars/postgres.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use datafusion::logical_expr::expr::ScalarFunction;
use pgrepr::compatible::server_version_with_build_info;

use crate::functions::FunctionNamespace;

Expand Down Expand Up @@ -475,3 +476,36 @@ impl BuiltinScalarUDF for PgArrayToString {
FunctionNamespace::Required("pg_catalog")
}
}

/// `pg_catalog.version()` implementation.
///
/// This provides more informatation that just the 'server_version' session
/// variable, and includes things like the build triple.
///
/// This uses a spoofed version (and so does not match the `version()` function)
/// since many postgres tools, including sqlalchemy, will check the version
/// against hard coded values.
#[derive(Clone, Copy, Debug)]
pub struct PgVersion;

impl ConstBuiltinFunction for PgVersion {
const NAME: &'static str = "version";
const DESCRIPTION: &'static str = "Returns the spoofed postgres version of the database";
const EXAMPLE: &'static str = "pg_catalog.version()";
const FUNCTION_TYPE: FunctionType = FunctionType::Scalar;
fn signature(&self) -> Option<Signature> {
Some(Signature::exact(vec![], Volatility::Stable))
}
}

impl BuiltinScalarUDF for PgVersion {
fn as_expr(&self, _: Vec<Expr>) -> Expr {
Expr::Literal(ScalarValue::Utf8(Some(
server_version_with_build_info().to_string(),
)))
}
Comment on lines +502 to +506
Copy link
Member Author

Choose a reason for hiding this comment

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

@universalmind303 I was originally trying to use session_var("server_version") here, but was getting a "Unknown variable: server_version" error.

Using server_version here would be incorrect anyways, but where would I look to enable using it with session_var?


fn namespace(&self) -> FunctionNamespace {
FunctionNamespace::Required("pg_catalog")
}
}
12 changes: 11 additions & 1 deletion testdata/sqllogictests/functions/version.slt
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
query B
query T
select starts_with(version(), 'v')
----
t

# There also exists a stub for pg_catalog.version(). This is hard coded to a
# postgres version maximize compatability with postgres tooling, and is
# _different_ than the `version()` function.
#
# Needed for great expections: <https://github.com/GlareDB/glaredb/issues/2436#issuecomment-1899383224>
query T
select starts_with(pg_catalog.version(), 'PostgreSQL 15.1')
----
t
9 changes: 9 additions & 0 deletions testdata/sqllogictests/vars.slt
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,12 @@ query T
show client_min_messages;
----
WARNING

# standard_conforming_strings

# TODO: Double check how we format bools for session vars. Interestingly,
# postgres returns 'on' for this.
query T
show standard_conforming_strings;
----
true