Skip to content

Commit

Permalink
refactor: remove location_prefix, which is no longer supported (#16757
Browse files Browse the repository at this point in the history 
)

* refactor: remove `location_prefix`

no longer supported

* tweak stateless test case

* tweak test cases

* tweak logic test
  • Loading branch information
@dantengsky
dantengsky authored Nov 5, 2024
1 parent 44668ed commit b89a604
Show file tree
Hide file tree
Showing 28 changed files with 32 additions and 233 deletions.
14 changes: 1 addition & 13 deletions src/query/ast/src/ast/statements/copy.rs
View file
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
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
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
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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
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
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
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

0 comments on commit b89a604

Please sign in to comment.