Skip to content

Commit

Permalink
fix(mssql): varchar params should not cause high cpu usage (#4918)
Browse files Browse the repository at this point in the history
  • Loading branch information
Weakky authored Jun 14, 2024
1 parent e9ac668 commit 9959b17
Show file tree
Hide file tree
Showing 10 changed files with 203 additions and 8 deletions.
2 changes: 1 addition & 1 deletion quaint/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,5 @@ pub use select::{DistinctType, Select};
pub use table::*;
pub use union::Union;
pub use update::*;
pub(crate) use values::Params;
pub use values::{IntoRaw, Raw, Value, ValueType, Values};
pub(crate) use values::{NativeColumnType, Params};
2 changes: 1 addition & 1 deletion quaint/src/ast/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
};
use std::borrow::Cow;

#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum TypeDataLength {
Constant(u16),
Maximum,
Expand Down
21 changes: 18 additions & 3 deletions quaint/src/ast/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,34 @@ where

/// A native-column type, i.e. the connector-specific type of the column.
#[derive(Debug, Clone, PartialEq)]
pub struct NativeColumnType<'a>(Cow<'a, str>);
pub struct NativeColumnType<'a> {
pub name: Cow<'a, str>,
pub length: Option<TypeDataLength>,
}

impl<'a> std::ops::Deref for NativeColumnType<'a> {
type Target = str;

fn deref(&self) -> &Self::Target {
&self.0
&self.name
}
}

impl<'a> From<&'a str> for NativeColumnType<'a> {
fn from(s: &'a str) -> Self {
Self(Cow::Owned(s.to_uppercase()))
Self {
name: Cow::Owned(s.to_uppercase()),
length: None,
}
}
}

impl<'a> From<(&'a str, Option<TypeDataLength>)> for NativeColumnType<'a> {
fn from((name, length): (&'a str, Option<TypeDataLength>)) -> Self {
Self {
name: Cow::Owned(name.to_uppercase()),
length,
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions quaint/src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,22 @@ pub trait Visitor<'a> {
Ok(())
}

fn visit_parameterized_text(&mut self, txt: Option<Cow<'a, str>>, nt: Option<NativeColumnType<'a>>) -> Result {
self.add_parameter(Value {
typed: ValueType::Text(txt),
native_column_type: nt,
});
self.parameter_substitution()?;

Ok(())
}

/// A visit to a value we parameterize
fn visit_parameterized(&mut self, value: Value<'a>) -> Result {
match value.typed {
ValueType::Enum(Some(variant), name) => self.visit_parameterized_enum(variant, name),
ValueType::EnumArray(Some(variants), name) => self.visit_parameterized_enum_array(variants, name),
ValueType::Text(txt) => self.visit_parameterized_text(txt, value.native_column_type),
_ => {
self.add_parameter(value);
self.parameter_substitution()
Expand Down
45 changes: 43 additions & 2 deletions quaint/src/visitor/mssql.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::Visitor;
use super::{NativeColumnType, Visitor};
#[cfg(any(feature = "postgresql", feature = "mysql"))]
use crate::prelude::{JsonArrayAgg, JsonBuildObject, JsonExtract, JsonType, JsonUnquote};
use crate::{
Expand All @@ -10,7 +10,7 @@ use crate::{
prelude::{Aliasable, Average, Query},
visitor, Value, ValueType,
};
use std::{convert::TryFrom, fmt::Write, iter};
use std::{borrow::Cow, convert::TryFrom, fmt::Write, iter};

static GENERATED_KEYS: &str = "@generated_keys";

Expand Down Expand Up @@ -176,6 +176,14 @@ impl<'a> Mssql<'a> {

Ok(())
}

fn visit_text(&mut self, txt: Option<Cow<'a, str>>, nt: Option<NativeColumnType<'a>>) -> visitor::Result {
self.add_parameter(Value {
typed: ValueType::Text(txt),
native_column_type: nt,
});
self.parameter_substitution()
}
}

impl<'a> Visitor<'a> for Mssql<'a> {
Expand Down Expand Up @@ -207,6 +215,39 @@ impl<'a> Visitor<'a> for Mssql<'a> {
self.parameters.push(value)
}

fn visit_parameterized_text(
&mut self,
txt: Option<Cow<'a, str>>,
nt: Option<NativeColumnType<'a>>,
) -> visitor::Result {
match nt {
Some(nt) => match (nt.name.as_ref(), nt.length) {
// Tiberius encodes strings as NVARCHAR by default. This causes implicit coercions which avoids using indexes.
// This cast ensures that VARCHAR instead.
("VARCHAR", length) => self.surround_with("CAST(", ")", |this| {
this.visit_text(txt, Some(nt))?;
this.write(" AS VARCHAR")?;

match length {
Some(TypeDataLength::Constant(length)) => {
this.write("(")?;
this.write(length)?;
this.write(")")?;
}
Some(TypeDataLength::Maximum) => {
this.write("(MAX)")?;
}
None => (),
}

Ok(())
}),
_ => self.visit_text(txt, Some(nt)),
},
nt => self.visit_text(txt, nt),
}
}

/// A point to modify an incoming query to make it compatible with the
/// SQL Server.
fn compatibility_modifications(&self, query: Query<'a>) -> Query<'a> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
mod mssql;
mod mysql;
mod postgres;
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use indoc::indoc;
use query_engine_tests::*;

#[test_suite(only(SqlServer))]
mod string {
fn schema_string() -> String {
let schema = indoc! {
r#"
model Parent {
#id(id, Int, @id)
vChar String @test.VarChar
vChar40 String @test.VarChar(40)
vCharMax String @test.VarChar(max)
}"#
};

schema.to_owned()
}

// Regression test for https://github.com/prisma/prisma/issues/17565
#[connector_test(schema(schema_string))]
async fn native_string(mut runner: Runner) -> TestResult<()> {
create_row(
&runner,
r#"{
id: 1,
vChar: "0"
vChar40: "0123456789012345678901234567890123456789"
vCharMax: "0123456789"
}"#,
)
.await?;

insta::assert_snapshot!(
run_query!(&runner, r#"{ findManyParent {
id
vChar
vChar40
vCharMax
}}"#),
@r###"{"data":{"findManyParent":[{"id":1,"vChar":"0","vChar40":"0123456789012345678901234567890123456789","vCharMax":"0123456789"}]}}"###
);

// VARCHAR
// Ensure the VarChar is casted to VARCHAR to avoid implicit coercion
runner.clear_logs().await;
insta::assert_snapshot!(
run_query!(&runner, r#"{ findManyParent(where: { vChar: "0" }) {
id
vChar
}}"#),
@r###"{"data":{"findManyParent":[{"id":1,"vChar":"0"}]}}"###
);
assert!(runner
.get_logs()
.await
.iter()
.any(|log| log.contains("WHERE [string_native_string].[Parent].[vChar] = CAST(@P1 AS VARCHAR)")));

// VARCHAR(40)
runner.clear_logs().await;
insta::assert_snapshot!(
run_query!(&runner, r#"{ findManyParent(where: { vChar40: "0123456789012345678901234567890123456789" }) {
id
vChar40
}}"#),
@r###"{"data":{"findManyParent":[{"id":1,"vChar40":"0123456789012345678901234567890123456789"}]}}"###
);

// Ensure the VarChar(40) is casted to VARCHAR(40) to avoid implicit coercion
assert!(runner
.get_logs()
.await
.iter()
.any(|log| log.contains("WHERE [string_native_string].[Parent].[vChar40] = CAST(@P1 AS VARCHAR(40))")));

// VARCHAR(MAX)
runner.clear_logs().await;
insta::assert_snapshot!(
run_query!(&runner, r#"{ findManyParent(where: { vCharMax: "0123456789" }) {
id
vCharMax
}}"#),
@r###"{"data":{"findManyParent":[{"id":1,"vCharMax":"0123456789"}]}}"###
);

// Ensure the VarChar is casted to VARCHAR(MAX) to avoid implicit coercion
assert!(runner
.get_logs()
.await
.iter()
.any(|log| log.contains("WHERE [string_native_string].[Parent].[vCharMax] = CAST(@P1 AS VARCHAR(MAX))")));

// Ensure it works as well with gt
runner.clear_logs().await;
insta::assert_snapshot!(
run_query!(&runner, r#"{ findManyParent(where: { vChar40: { gt: "0" } }) { id vChar40 } }"#),
@r###"{"data":{"findManyParent":[{"id":1,"vChar40":"0123456789012345678901234567890123456789"}]}}"###
);
assert!(runner
.get_logs()
.await
.iter()
.any(|log| log.contains("WHERE [string_native_string].[Parent].[vChar40] > CAST(@P1 AS VARCHAR(40))")));

Ok(())
}

async fn create_row(runner: &Runner, data: &str) -> TestResult<()> {
runner
.query(format!("mutation {{ createOneParent(data: {}) {{ id }} }}", data))
.await?
.assert_success();
Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -331,4 +331,8 @@ impl TestLogCapture {

logs
}

pub async fn clear_logs(&mut self) {
while self.rx.try_recv().is_ok() {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,10 @@ impl Runner {
}
}

pub async fn clear_logs(&mut self) {
self.log_capture.clear_logs().await
}

pub fn connector_version(&self) -> &ConnectorVersion {
&self.version
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ impl ScalarFieldExt for ScalarField {
},
};

value.with_native_column_type(self.native_type().map(|nt| nt.name()))
let nt_col_type = self.native_type().map(|nt| (nt.name(), parse_scalar_length(self)));

value.with_native_column_type(nt_col_type)
}

fn type_family(&self) -> TypeFamily {
Expand Down

0 comments on commit 9959b17

Please sign in to comment.