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.

Sign up for GitHub

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

refactor: remove location_prefix, which is no longer supported #16757

Merged
merged 4 commits into from
Nov 5, 2024
Merged

refactor: remove location_prefix, which is no longer supported #16757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c939f68
refactor: remove `location_prefix`
dantengsky Nov 4, 2024
3a78a09
tweak stateless test case
dantengsky Nov 4, 2024
32ec2cc
tweak test cases
dantengsky Nov 4, 2024
21e8e25
tweak logic test
dantengsky Nov 4, 2024
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
14 changes: 1 addition & 13 deletions src/query/ast/src/ast/statements/copy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,6 @@ pub struct UriLocation {
pub protocol: String,
pub name: String,
pub path: String,
pub part_prefix: String,
pub connection: Connection,
}

Expand All @@ -351,23 +350,17 @@ impl UriLocation {
protocol: String,
name: String,
path: String,
part_prefix: String,
conns: BTreeMap<String, String>,
) -> Self {
Self {
protocol,
name,
path,
part_prefix,
connection: Connection::new(conns),
}
}

pub fn from_uri(
uri: String,
part_prefix: String,
conns: BTreeMap<String, String>,
) -> Result<Self> {
pub fn from_uri(uri: String, conns: BTreeMap<String, String>) -> Result<Self> {
// fs location is not a valid url, let's check it in advance.
if let Some(path) = uri.strip_prefix("fs://") {
if !path.starts_with('/') {
Expand All @@ -380,7 +373,6 @@ impl UriLocation {
"fs".to_string(),
"".to_string(),
path.to_string(),
part_prefix,
BTreeMap::default(),
));
}
Expand Down Expand Up @@ -411,7 +403,6 @@ impl UriLocation {
protocol,
name,
path,
part_prefix,
connection: Connection::new(conns),
})
}
Expand All @@ -428,9 +419,6 @@ impl Display for UriLocation {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
write!(f, "'{}://{}{}'", self.protocol, self.name, self.path)?;
write!(f, "{}", self.connection)?;
if !self.part_prefix.is_empty() {
write!(f, " LOCATION_PREFIX = '{}'", self.part_prefix)?;
}
Ok(())
}
}
Expand Down
12 changes: 3 additions & 9 deletions src/query/ast/src/parser/stage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,28 +221,22 @@ pub fn string_location(i: Input) -> IResult<FileLocation> {
rule! {
#literal_string
~ (CONNECTION ~ ^"=" ~ ^#connection_options ~ ","?)?
~ (LOCATION_PREFIX ~ ^"=" ~ ^#literal_string ~ ","?)?
},
|(location, connection_opts, location_prefix)| {
|(location, connection_opts)| {
if let Some(stripped) = location.strip_prefix('@') {
if location_prefix.is_none() && connection_opts.is_none() {
if connection_opts.is_none() {
Ok(FileLocation::Stage(stripped.to_string()))
} else {
Err(nom::Err::Failure(ErrorKind::Other(
"uri location should not start with '@'",
)))
}
} else {
let part_prefix = if let Some((_, _, p, _)) = location_prefix {
p
} else {
"".to_string()
};
// fs location is not a valid url, let's check it in advance.

let conns = connection_opts.map(|v| v.2).unwrap_or_default();

let uri = UriLocation::from_uri(location, part_prefix, conns)
let uri = UriLocation::from_uri(location, conns)
.map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid uri")))?;
Ok(FileLocation::Uri(uri))
}
Expand Down
5 changes: 0 additions & 5 deletions src/query/ast/tests/it/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ fn test_statement() {
r#"create table a.b like c.d;"#,
r#"create table t like t2 engine = memory;"#,
r#"create table if not exists a.b (a int) 's3://testbucket/admin/data/' connection=(aws_key_id='minioadmin' aws_secret_key='minioadmin' endpoint_url='http://127.0.0.1:9900');"#,
r#"
create table if not exists a.b (a int) 's3://testbucket/admin/data/'
connection=(aws_key_id='minioadmin' aws_secret_key='minioadmin' endpoint_url='http://127.0.0.1:9900')
location_prefix = 'db';
"#,
r#"truncate table a;"#,
r#"truncate table "a".b;"#,
r#"drop table a;"#,
Expand Down
2 changes: 1 addition & 1 deletion src/query/ast/tests/it/testdata/stmt-error.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ error:
--> SQL:1:38
|
1 | COPY INTO mytable FROM 's3://bucket' CONECTION= ();
| ^^^^^^^^^ unexpected `CONECTION`, expecting `CONNECTION`, `ON_ERROR`, `RETURN_FAILED_ONLY`, `LOCATION_PREFIX`, `FORMAT`, `VALIDATION_MODE`, `FORCE`, `PATTERN`, `FILES`, `PURGE`, `SIZE_LIMIT`, `FILE_FORMAT`, `MAX_FILES`, `DISABLE_VARIANT_CHECK`, `SPLIT_SIZE`, or `;`
| ^^^^^^^^^ unexpected `CONECTION`, expecting `CONNECTION`, `ON_ERROR`, `RETURN_FAILED_ONLY`, `FORMAT`, `VALIDATION_MODE`, `FORCE`, `PATTERN`, `FILES`, `PURGE`, `SIZE_LIMIT`, `FILE_FORMAT`, `MAX_FILES`, `DISABLE_VARIANT_CHECK`, `SPLIT_SIZE`, or `;`


---------- Input ----------
Expand Down
91 changes: 0 additions & 91 deletions src/query/ast/tests/it/testdata/stmt.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2759,81 +2759,6 @@ CreateTable(
protocol: "s3",
name: "testbucket",
path: "/admin/data/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
"aws_key_id": "minioadmin",
"aws_secret_key": "minioadmin",
"endpoint_url": "http://127.0.0.1:9900",
},
},
},
),
cluster_by: None,
table_options: {},
as_query: None,
table_type: Normal,
},
)


---------- Input ----------
create table if not exists a.b (a int) 's3://testbucket/admin/data/'
connection=(aws_key_id='minioadmin' aws_secret_key='minioadmin' endpoint_url='http://127.0.0.1:9900')
location_prefix = 'db';
---------- Output ---------
CREATE TABLE IF NOT EXISTS a.b (a Int32) 's3://testbucket/admin/data/' CONNECTION = ( aws_key_id = 'minioadmin', aws_secret_key = 'minioadmin', endpoint_url = 'http://127.0.0.1:9900' ) LOCATION_PREFIX = 'db'
---------- AST ------------
CreateTable(
CreateTableStmt {
create_option: CreateIfNotExists,
catalog: None,
database: Some(
Identifier {
span: Some(
27..28,
),
name: "a",
quote: None,
ident_type: None,
},
),
table: Identifier {
span: Some(
29..30,
),
name: "b",
quote: None,
ident_type: None,
},
source: Some(
Columns(
[
ColumnDefinition {
name: Identifier {
span: Some(
32..33,
),
name: "a",
quote: None,
ident_type: None,
},
data_type: Int32,
expr: None,
comment: None,
},
],
None,
),
),
engine: None,
uri_location: Some(
UriLocation {
protocol: "s3",
name: "testbucket",
path: "/admin/data/",
part_prefix: "db",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -11299,7 +11224,6 @@ CreateStage(
protocol: "s3",
name: "load",
path: "/files/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -11344,7 +11268,6 @@ CreateStage(
protocol: "s3",
name: "load",
path: "/files/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -11389,7 +11312,6 @@ CreateStage(
protocol: "azblob",
name: "load",
path: "/files/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -11434,7 +11356,6 @@ CreateStage(
protocol: "azblob",
name: "load",
path: "/files/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -14050,7 +13971,6 @@ CopyIntoTable(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -14127,7 +14047,6 @@ CopyIntoTable(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -14206,7 +14125,6 @@ CopyIntoTable(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -14287,7 +14205,6 @@ CopyIntoTable(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -14358,7 +14275,6 @@ CopyIntoTable(
protocol: "https",
name: "127.0.0.1:9900",
path: "/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -14414,7 +14330,6 @@ CopyIntoTable(
protocol: "https",
name: "127.0.0.1",
path: "/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -14562,7 +14477,6 @@ CopyIntoLocation(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -14728,7 +14642,6 @@ CopyIntoTable(
protocol: "s3",
name: "mybucket",
path: "/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -15009,7 +14922,6 @@ CopyIntoTable(
protocol: "fs",
name: "",
path: "/path/to/data.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -15084,7 +14996,6 @@ CopyIntoTable(
protocol: "s3",
name: "databend",
path: "/books.csv",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down Expand Up @@ -17249,7 +17160,6 @@ Query(
protocol: "s3",
name: "test",
path: "/bucket",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {},
Expand Down Expand Up @@ -22350,7 +22260,6 @@ AttachTable(
protocol: "s3",
name: "a",
path: "/",
part_prefix: "",
connection: Connection {
visited_keys: {},
conns: {
Expand Down
1 change: 0 additions & 1 deletion src/query/ee/src/attach_table/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ impl AttachTableHandler for RealAttachTableHandler {
schema: Arc::new(snapshot.schema.clone()),
engine: plan.engine.to_string(),
storage_params: plan.storage_params.clone(),
part_prefix: plan.part_prefix.clone(),
options,
default_cluster_key: None,
field_comments,
Expand Down
1 change: 0 additions & 1 deletion src/query/ee/tests/it/inverted_index/pruning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ async fn test_block_pruner() -> Result<()> {
engine: Engine::Fuse,
engine_options: Default::default(),
storage_params: None,
part_prefix: "".to_string(),
options: [
(FUSE_OPT_KEY_ROW_PER_BLOCK.to_owned(), num_blocks_opt),
(FUSE_OPT_KEY_BLOCK_PER_SEGMENT.to_owned(), "5".to_owned()),
Expand Down
1 change: 0 additions & 1 deletion src/query/service/src/interpreters/interpreter_table_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ impl CreateTableInterpreter {
schema: schema.clone(),
engine: self.plan.engine.to_string(),
storage_params: self.plan.storage_params.clone(),
part_prefix: self.plan.part_prefix.clone(),
options,
engine_options: self.plan.engine_options.clone(),
default_cluster_key: None,
Expand Down
1 change: 0 additions & 1 deletion src/query/service/src/pipelines/processors/transforms/transform_recursive_cte_source.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ async fn create_memory_table_for_cte_scan(
engine: Engine::Memory,
engine_options: Default::default(),
storage_params: None,
part_prefix: "".to_string(),
options: Default::default(),
field_comments: vec![],
cluster_key: None,
Expand Down
14 changes: 4 additions & 10 deletions src/query/service/src/table_functions/infer_schema/parquet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,8 @@ impl AsyncSource for ParquetInferSchemaSource {
FileLocation::Stage(location.to_string())
} else if let Some(connection_name) = &self.args_parsed.connection_name {
let conn = self.ctx.get_connection(connection_name).await?;
let uri = UriLocation::from_uri(
self.args_parsed.location.clone(),
"".to_string(),
conn.storage_params,
)?;
let uri =
UriLocation::from_uri(self.args_parsed.location.clone(), conn.storage_params)?;
let proto = conn.storage_type.parse::<Scheme>()?;
if proto != uri.protocol.parse::<Scheme>()? {
return Err(ErrorCode::BadArguments(format!(
Expand All @@ -92,11 +89,8 @@ impl AsyncSource for ParquetInferSchemaSource {
}
FileLocation::Uri(uri)
} else {
let uri = UriLocation::from_uri(
self.args_parsed.location.clone(),
"".to_string(),
BTreeMap::default(),
)?;
let uri =
UriLocation::from_uri(self.args_parsed.location.clone(), BTreeMap::default())?;
FileLocation::Uri(uri)
};
let (stage_info, path) = resolve_file_location(self.ctx.as_ref(), &file_location).await?;
Expand Down
Loading
Loading