Conversation
Pull reviewers statsStats of the last 14 days for rrdb:
|
코드 리뷰 요약Walkthrough프로젝트 아키텍처를 크게 개선하는 리팩토링입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DBEngine
participant StorageEncoder
participant FileSystem
Client->>DBEngine: desc_table(DescTableQuery)
DBEngine->>StorageEncoder: decode(table.config)
StorageEncoder->>FileSystem: read(table.config)
FileSystem-->>StorageEncoder: config bytes
StorageEncoder-->>DBEngine: TableSchema
DBEngine->>DBEngine: construct ExecuteResult<br/>(Field, Type, Null, Default, Comment)
DBEngine-->>Client: Result<ExecuteResult>
Client->>DBEngine: show_databases()
DBEngine->>FileSystem: read_dir(data_dir)
par Parallel Reads
DBEngine->>StorageEncoder: decode(database.config)
StorageEncoder->>FileSystem: read(database.config)
FileSystem-->>StorageEncoder: config bytes
StorageEncoder-->>DBEngine: DatabaseSchema
end
DBEngine->>DBEngine: collect database names
DBEngine-->>Client: Result<ExecuteResult>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 특별 주의 영역:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Total Coverage: 54.50% |
myyrakle
force-pushed
the
refac/#176
branch
2 times, most recently
from
4432707 to
ddf61f9
Compare
|
✅ Total Coverage: 54.48% |
|
✅ Total Coverage: 54.48% |
|
✅ Total Coverage: 54.48% |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/engine/parser/implements/dml/select.rs (1)
191-196: E0332 에러 메시지에서 잘못된 변수 출력 (디버깅 혼동 가능).
GROUP BY검증 로직에서 집계 컬럼이group_by_columns에 포함된 경우 에러를 내는데, 에러 메시지에는 실제로 문제가 된 개별 컬럼(aggregate_column) 대신group_by_columns전체를 출력하고 있습니다.for aggregate_column in aggregate_columns { if group_by_columns.contains(&aggregate_column) { return Err(ParsingError::wrap(format!( "E0332: column '{:?}' cannot be in a GROUP BY clause", group_by_columns ))); } }디버깅 편의를 위해서는 아래처럼
aggregate_column을 출력하는 편이 더 자연스럽고, 메시지와 실제 문제 대상이 정확히 일치합니다.- return Err(ParsingError::wrap(format!( - "E0332: column '{:?}' cannot be in a GROUP BY clause", - group_by_columns - ))); + return Err(ParsingError::wrap(format!( + "E0332: column '{:?}' cannot be in a GROUP BY clause", + aggregate_column + )));동작에는 영향이 없지만, 에러 해석 시 혼동을 줄이기 위해 한 번 손보시는 것을 추천드립니다.
src/engine/parser/implements/ddl/top_level.rs (1)
60-65: DROP 핸들러의 에러 메시지 내용이 실제 명령과 맞지 않습니다.
handle_drop_query에서 지원하지 않는 명령일 때"possible commands: (create table)"라고 안내하고 있어 사용자가 DROP 문에서 헷갈릴 수 있습니다. 아래처럼 텍스트를 DROP 에 맞게 바꾸는 것을 제안합니다.- _ => Err(ParsingError::wrap( - "E1106 not supported command. possible commands: (create table)", - )), + _ => Err(ParsingError::wrap( + "E1106 not supported command. possible commands: (drop table, drop database)", + )),(정확한 지원 범위에 맞춰 문구는 조정해 주시면 될 것 같습니다.)
src/engine/actions/ddl/alter_database.rs (1)
63-76: config 쓰기 실패 시 에러 메시지가 실제 원인과 어긋납니다.if let Err(_error) = tokio::fs::write(...).await { return Err(ExecuteError::wrap("no database name")); }부분에서, 실제로는
database.config파일 쓰기 실패인데 메시지는"no database name"으로 되어 있어 디버깅 시 혼란을 줄 수 있습니다. 또한database_config: Option<DatabaseSchema>변수를 그대로 두고 있어 타입과 이름이 어울리지 않는 부분도 살짝 거슬립니다.예를 들어 다음처럼 고치는 것을 고려해 볼 수 있습니다.
- let database_config: Option<DatabaseSchema> = + let database_schema_opt: Option<DatabaseSchema> = encoder.decode(data.as_slice()); @@ - match database_config { - Some(mut database_config) => { - database_config.database_name = to_database_name; - if let Err(_error) = tokio::fs::write( + match database_schema_opt { + Some(mut database_schema) => { + database_schema.database_name = to_database_name; + if let Err(error) = tokio::fs::write( config_path, - encoder.encode(database_config), + encoder.encode(database_schema), ) .await { - return Err(ExecuteError::wrap("no database name")); + return Err(ExecuteError::wrap(format!( + "database config write failed: {}", + error + ))); }이렇게 하면 에러 메시지가 실제 실패 지점과 맞아떨어지고, 변수명도 타입과 더 잘 대응됩니다.
src/engine/actions/ddl/create_table.rs (1)
17-19:query.table및database_name에 대한unwrap()체인 제거 필요create_table.rs 17-19 라인의 unwrap() 사용이 실제로 확인되었습니다:
CreateTableQuery.table은Option<TableName>타입TableName.database_name은Option<String>타입- 테스트 코드에서 database_name을
None으로 설정하는 경우 존재같은 레이어의 create_database.rs에서는 이미
ok_or_else(|| ExecuteError::wrap(...))?패턴으로 안전하게 처리하고 있으므로, 동일한 방식으로 수정이 필요합니다. 단, 수정 후query.into()를 호출할 때TableSchema::from()의query.table.unwrap()도 고려하여, 이미 추출된 table 정보를 직접 사용하거나 From impl을 함께 검토하는 것을 권장합니다.src/engine/actions/dml/insert.rs (1)
88-126:columns_map.get(...).unwrap()호출로 인한 런타임 패닉 가능성 제거 필요검증 결과, 리뷰 의견이 정확합니다. 다음 두 위치에서 동일한 패턴이 확인되었습니다:
- Line 90:
let column_config_info = columns_map.get(column_name).unwrap();
- Line 101의 match 문에서 None 분기가 도달 불가능 상태
- Line 140:
let column_config_info = columns_map.get(column_name).unwrap();
- Line 161의 match 문에서 None 분기가 도달 불가능 상태
컬럼이 존재하지 않는 경우,
.unwrap()에서 먼저 패닉이 발생하므로 아래의 명시적 에러 처리 분기는 절대 실행되지 않습니다. 제안된 대로 match 문으로 통합하여 일관된 에러 처리(ExecuteError 반환)를 구현하시기 바랍니다.- for (i, column_name) in query.columns.iter().enumerate() { - let column_config_info = columns_map.get(column_name).unwrap(); + for (i, column_name) in query.columns.iter().enumerate() { + let column_config_info = match columns_map.get(column_name) { + Some(info) => info, + None => { + return Err(ExecuteError::wrap(format!( + "column '{}' not exists", + column_name + ))); + } + };라인 140부터 시작하는
remain_columns루프도 동일하게 수정이 필요합니다.src/engine/initialize.rs (1)
59-69:if let Err(error) = ... && error.kind()패턴은 현재 안정적인 Rust에서 컴파일되지 않습니다.웹 조사 결과, if-let guards 기능은 2025년 11월 현재 안정화되지 않았으며 Lang-team FCP 논의 단계에 있습니다. 코드의 모든 위치(59-69, 96-104, 108-116, 156-169줄)에서 이 불안정한 기능을 사용하고 있으므로 안정적인 Rust에서는 컴파일되지 않습니다.
제시된 중첩 if-let 해결책이 올바른 수정 방법입니다:
if let Err(error) = self.file_system.create_dir(base_path).await { if error.kind() != std::io::ErrorKind::AlreadyExists { println!("path {:?}", base_path); println!("error: {:?}", error.to_string()); return Err(ExecuteError::wrap(error.to_string())); } }
create_data_directory_if_not_exists,create_wal_directory_if_not_exists,write_and_check_err모두 동일한 방식으로 수정이 필요합니다.src/engine/actions/dml/delete.rs (1)
52-55:ScanType::IndexScan분기에서unimplemented!()호출은 런타임 패닉을 유발할 수 있습니다
ScanType::IndexScan(_index) => { unimplemented!() }로 되어 있어, 옵티마이저가 인덱스 스캔 플랜을 생성하는 순간 프로세스가 패닉으로 종료됩니다.인덱스 스캔이 아직 미구현 상태라면:
- 최소한
ExecuteError::wrap("index scan for DELETE is not implemented yet")형태의 에러를 반환하도록 바꾸거나,- 옵티마이저 단계에서 인덱스 스캔 플랜 자체를 생성하지 않도록 막는 편이 안전합니다.
현재처럼
unimplemented!()를 남겨두면, 실제 환경에서 특정 쿼리만 프로세스를 전부 죽여버리는 상황이 생길 수 있습니다.
🧹 Nitpick comments (17)
src/engine/parser/implements/debug.rs (1)
1-1: 임포트 경로 업데이트가 리팩토링 목표와 일치합니다.파서 모듈을
engine네임스페이스 아래로 이동하는 것이 프로젝트 재구성 목표와 잘 맞습니다.참고: 모듈 이름
predule이prelude의 오타일 가능성이 있습니다 (Rust에서 일반적인 컨벤션). 그러나 이것이 의도적인 이름이거나 코드베이스 전체에서 일관되게 사용되고 있다면 무시하셔도 됩니다.src/engine/actions/dml/scan.rs (1)
20-21:database_name.unwrap()대신 명시적인 에러로 변환하는 방안을 고려해보면 좋겠습니다.
table_name.database_name.unwrap()는 호출 시점에 DB 이름이 항상 Some 이라는 강한 인바리언트를 전제로 합니다. 파서/플래너 단계에서 이 보장이 깨질 여지가 조금이라도 있다면, 여기에서 패닉 대신ExecuteError/RRDBError로 변환해 반환하는 쪽이 디버깅과 안정성 측면에서 더 안전합니다.src/engine/ast/types/expression.rs (1)
330-353: 테스트에서 fully-qualified 경로 대신 이미 import 된 타입명을 사용하면 가독성이 더 좋아집니다.테스트 모듈 상단에서
BetweenExpression,NotBetweenExpression,ParenthesesExpression를 import 하고 있으므로, 본문에서는
crate::engine::ast::dml::expressions::between::BetweenExpression { ... }crate::engine::ast::dml::expressions::not_between::NotBetweenExpression { ... }crate::engine::ast::dml::expressions::parentheses::ParenthesesExpression { ... }대신 각각
BetweenExpression { ... },NotBetweenExpression { ... },ParenthesesExpression { ... }형태로 써 주면 라인이 짧아지고 변경 시 중복 수정도 줄어듭니다.Also applies to: 341-345, 352-357
src/engine/parser/implements/tcl/begin.rs (1)
1-6: 엔진 네임스페이스로의 경로 이관은 자연스럽습니다.
BeginTransactionQuery,SQLStatement,Token,Parser/ParserContext를 모두crate::engine::...아래로 옮긴 건 전체 레이아웃 리팩토링 방향과 잘 맞습니다.한 가지 정도만 참고용으로 말씀드리면, 이 파일은
Token을lexer::tokens::Token에서 직접 가져오고, 다른 구현(src/engine/parser/implements/other/use.rs등)은lexer::predule::Token을 쓰고 있어서 경로 스타일이 조금씩 다릅니다. 추후에 정리하실 때 prelude 경로를 통일해 두면 모듈 구조를 추적하기 조금 더 수월해질 것 같습니다.src/engine/parser/implements/other/use.rs (1)
1-6: USE 구문 파서의 엔진 경로 이관은 적절합니다.
UseDatabaseQuery,SQLStatement,Token,Parser/ParserContext를crate::engine::...아래로 옮긴 부분이 전체 레이아웃과 잘 맞고,parse_use_query구현은 그대로라 동작 변화는 없어 보입니다.추가로, 이 파일은
Token을lexer::predule::Token에서 가져오고,tcl/begin.rs는lexer::tokens::Token을 직접 import 하고 있어 prelude 사용 여부가 파일마다 조금 다릅니다. 장기적으로는 prelude 사용 여부를 한쪽으로 통일해 두면 추후 리팩토링 시 추적이 더 쉬울 것 같습니다.src/engine/actions/ddl/drop_database.rs (1)
3-8: DBEngine 이전 및 결과 스키마는 문제 없어 보입니다 (사소한 최적화 여지).
Executor→DBEngine이전과engine::ast,engine::types사용으로 네임스페이스 정리는 잘 된 것 같습니다. 동작도 기존과 동일하게 유지되는 것으로 보입니다.사소한 부분으로는:
tokio::fs::remove_dir_all(database_path.clone()).await에서clone()없이 참조를 넘겨도 됩니다.처럼 불필요한 복사를 줄여서 조금 더 깔끔하게 정리할 수 있습니다.
Also applies to: 10-12, 36-47
src/engine/actions/ddl/alter_table.rs (2)
14-24:query.action매치 이후query.table.unwrap()사용 패턴은 소유권 측면에서 위험합니다.현재 구현은 다음과 같은 구조입니다:
- Line 20-24:
let TableName { .. } = query.table.clone().unwrap();- Line 30:
match query.action { ... }- 각 분기 내부에서 여러 번
self.get_table_config(query.table.unwrap()).await?;호출Rust 소유권 규칙상,
match query.action으로query.action필드를 move 한 뒤에 같은query의 다른 필드(query.table)를 사용하는 패턴은 컴파일 에러(부분 move)로 이어질 수 있습니다. 현재 코드가 실제로 빌드되는지, 혹은 이 부분이 다른 타입 정의/파생 구현에 의존해서 우연히 통과하고 있는지 꼭 한 번 확인하는 편이 좋겠습니다.보다 안전하고 명확한 방법은, 초기에 한 번만
table값을 꺼내서 재사용하는 것입니다. 예를 들어:- let TableName { - database_name, - table_name, - } = query.table.clone().unwrap(); + let table = query.table.clone().ok_or_else(|| ExecuteError::wrap("no table"))?; + let TableName { + database_name, + table_name, + } = table.clone(); ... - let mut table_config = self.get_table_config(query.table.unwrap()).await?; + let mut table_config = self.get_table_config(Some(table.clone())).await?;처럼
table을 별도 변수로 유지하고, 이후에는query.table.unwrap()대신 그 변수를 넘기면 소유권 문제가 사라지고 의도도 더 분명해집니다.
또한 TableConfig → TableSchema 변경(예: Line 121, 173, 225, 277)은 encoder가 동일 타입으로 encode하고 있는지만 확인되면 크게 문제 없어 보입니다.Also applies to: 30-32, 46-49, 63-66, 90-93, 121-122, 173-174, 225-226, 277-278
121-155: 반복되는 config read/modify/write 로직은 헬퍼로 추출하면 유지보수가 쉬워집니다.
AlterColumnDropDefault,AlterColumnSetNotNull,AlterColumnDropNotNull,AlterColumnSetType네 분기에서 모두:
tokio::fs::read(config_path).awaitencoder.decode::<Option<TableSchema>>(...)columns.iter_mut().find(|e| e.name == column_name)- 값 수정 후 다시 encode + write
같은 패턴을 거의 그대로 반복하고 있습니다. 기능상 문제는 없어 보이지만, 향후 schema 구조 변경이나 에러 메시지 개선 시 네 군데를 모두 수정해야 해서 유지보수 비용이 큽니다.
예를 들어 “컬럼을 찾고 수정한 뒤 table.config를 다시 저장하는” 공통 헬퍼를 하나 두고, 각 분기에서는 “무엇을 어떻게 수정할지”만 클로저로 넘기는 형태로 정리하면 가독성과 변경 용이성이 좋아질 것 같습니다.
Also applies to: 173-207, 225-259, 277-311
src/engine/actions/dml/insert.rs (1)
35-45:TableSchema디코드 변경은 일관되어 보이지만 encoder 쪽 타입도 같이 맞춰야 합니다.Line 37에서
let table_config: Option<TableSchema> = encoder.decode(data.as_slice());로 타입을 바꾼 것은 전체적인TableConfig→TableSchema마이그레이션과 잘 맞습니다. 다만:
- 같은 encoder가 다른 곳에서도
TableSchema기준으로 encode하고 있는지,- 기존
table.config파일과의 호환성이 필요한지는 한번 더 확인해 보시는 게 좋겠습니다. 새로운 버전만 고려한다면 크게 문제는 없어 보입니다.
src/engine/initialize.rs (2)
79-94:create_global_config_if_not_exists가 항상 덮어쓰기 하는 점은 의도 확인이 필요합니다.함수 이름은 “if_not_exists”인데, 실제 구현은:
- 파일 존재 여부를 확인하지 않고,
- 매 호출마다
LaunchConfig::default()를 TOML로 직렬화하여 그대로 덮어씁니다.이미 사용자가 수정한 설정이 있을 수 있는 시나리오까지 고려한다면, 최초 1회만 생성하고 이후에는 덮어쓰지 않도록
read/metadata등을 확인하는 로직이 필요한지 한 번 검토해 보시는 것이 좋겠습니다.
195-201: 데몬 시작 시 프로세스 실행 실패만 확인하고 exit code는 무시하고 있습니다.
check_output_status는 현재:if output.is_err() { Err(ExecuteError::wrap("failed to start daemon")) } else { Ok(()) }으로, 커맨드 실행 자체가 실패했는지만 보고 실제
status.success()여부는 확인하지 않습니다.서비스 등록/시작 커맨드가 비정상 종료(exit code ≠ 0) 하는 경우까지 감지하고 싶다면,
Output가 Ok일 때 추가로status.success()를 체크하는 것이 더 견고할 것 같습니다.src/engine/actions/ddl/drop_table.rs (1)
3-8: DBEngine 기반drop_table리팩토링은 무난해 보입니다.
Executor→DBEngine,engine::ast/engine::types경로 정리 모두 일관되게 적용되어 있고, 에러 처리와 결과 포맷도 기존과 동일한 것으로 보입니다.Optional하게는,
tokio::fs::remove_dir_all호출 시base_path.clone()대신 참조를 넘겨 불필요한 복사를 줄일 수 있습니다.Also applies to: 10-12, 36-47
src/engine/actions/dml/delete.rs (1)
26-40: Delete 플랜에서table_infos가 사용되지 않아 불필요한 IO가 발생합니다
DeletePlanItem::DeleteFrom분기에서:
self.get_table_config(table_name.clone()).await?호출로 디스크에서 스키마를 읽어오고,- 그 결과를
table_infos.push(table_config);로 누적하지만,이후
table_infos는 어디에서도 사용되지 않습니다. 현재 구현 기준으로는 삭제 대상 로우를 수집하는 데에만 집중하고 있어, 이 호출은 검증/스키마 활용 없이 단순히 비용만 증가시키는 형태입니다.
- 정말 스키마 정보가 필요 없다면,
get_table_config호출과table_infos벡터 자체를 제거하는 쪽이 더 낫고,- 향후
ReduceContext.config_columns등에서 스키마를 활용할 계획이라면, 그 용도에 맞게 실제로 사용되도록 연결해 두는 편이 좋습니다.src/engine/actions/dml/update.rs (2)
64-75: Update 필터 단계에서total_count: 0은 delete 구현과 일관되지 않습니다
UpdatePlanItem::Filter에서ReduceContext를 생성할 때:let reduce_context = ReduceContext { row: Some(row.to_owned()), table_alias_map, config_columns: vec![], total_count: 0, };으로 고정 0을 넘기고 있습니다. 반면
delete구현에서는 같은 필터링 단계에서total_count = rows.len()을 사용합니다.
reduce_expression이total_count를 활용하는 표현식(예: 전체 행 수 기반 계산)을 지원한다면, 두 연산(Update/Delete)의 동작이 일관되지 않을 수 있습니다. 특별한 이유가 없다면, 여기서도rows.len()값을 전달하거나, 최소한 주석으로 설계 의도를 명시해 두는 편이 좋겠습니다.
116-132: SET 표현식에서 기존 행(row) 참조가 필요한지 여부를 명확히 할 필요가 있습니다Update 적용 루프에서:
let reduce_context = ReduceContext { row: None, table_alias_map: table_alias_map.clone(), config_columns: config_columns.clone(), total_count: 0, }; for update_item in &update_items { let set_value = self .reduce_expression(set_value, reduce_context.clone()) .await?; // ... }
Filter단계에서는row: Some(row.to_owned())를 넘기지만,SET표현식 평가 시에는row: None으로 전달하고 있습니다.
- 만약
SET col = col + 1같이 기존 행의 값을 참조하는 표현식을 지원할 계획이라면, 여기서도 현재 행을ReduceContext.row에 넣어주는 것이 자연스럽고,- 반대로 Update가 상수/리터럴/함수 호출만 허용되고 기존 행 참조를 명시적으로 금지하는 설계라면, 코드나 타입/도큐먼트로 그 제약을 표현해 두는 편이 오해를 줄일 수 있습니다.
현재 설계 의도가 어느 쪽인지 한 번 점검해 보시는 것이 좋겠습니다.
src/engine/actions/etc.rs (2)
96-145:show_databases/show_tables의std::fs::read_dir역시 비동기 환경에서 블로킹입니다두 함수 모두
async fn이지만, 디렉터리 나열에는 동기식std::fs::read_dir를 사용하고 있습니다:match std::fs::read_dir(&base_path) { ... } match std::fs::read_dir(&tables_path) { ... }목록이 매우 많은 경우 이 호출도 꽤 시간이 걸릴 수 있고, 해당 async 함수가 실행되는 런타임 스레드를 블로킹하게 됩니다. 가능하다면:
tokio::fs::read_dir+ 비동기 이터레이션을 사용하거나,- 또는
tokio::task::spawn_blocking으로 동기 디렉터리 탐색을 별도 스레드로 분리하는 방식을 고려해 볼 수 있습니다. 현재 규모에서 문제가 되지 않더라도, 데이터베이스/테이블 수가 늘어날 경우 병목이 될 수 있는 부분입니다.
224-237:use_databases가 실제 엔진 상태를 변경하지 않는 점에 대한 의도 공유가 있으면 좋겠습니다현재
use_databases는 다음과 같이 단순 메시지만 반환합니다:Ok(ExecuteResult { columns: vec![...], rows: vec![ExecuteRow { fields: vec![ExecuteField::String(format!( "database changed: {}", query.database_name ))], }], })
DBEngine내부에는 선택된 데이터베이스를 나타내는 상태 필드가 없어서, 이 함수가 이후 쿼리의 기본 데이터베이스 컨텍스트에 영향을 주지는 않는 것으로 보입니다.
- 클라이언트/상위 레이어에서
USE명령을 해석해 별도의 세션 상태를 관리하고, 여기서는 단순히 사용자에게 메시지만 돌려주는 구조라면 지금 구현도 일관된 선택이고,- 그렇지 않고 엔진 레벨에서 현재 데이터베이스를 기억해야 한다면,
DBEngine에 세션/컨텍스트 상태를 추가하거나,LaunchConfig/연결 단위 상태와 연계하는 설계가 필요해 보입니다.의도가 어느 쪽인지 주석이나 설계 문서로 한 번 명시해 두면 이후 유지보수 시 혼동을 줄일 수 있을 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
src/common/command.rs(1 hunks)src/common/fs.rs(0 hunks)src/common/mod.rs(1 hunks)src/config/launch_config.rs(3 hunks)src/config/mod.rs(1 hunks)src/engine/actions/ddl/alter_database.rs(3 hunks)src/engine/actions/ddl/alter_table.rs(5 hunks)src/engine/actions/ddl/create_database.rs(3 hunks)src/engine/actions/ddl/create_table.rs(2 hunks)src/engine/actions/ddl/drop_database.rs(1 hunks)src/engine/actions/ddl/drop_table.rs(1 hunks)src/engine/actions/dml/delete.rs(1 hunks)src/engine/actions/dml/insert.rs(4 hunks)src/engine/actions/dml/scan.rs(1 hunks)src/engine/actions/dml/select.rs(2 hunks)src/engine/actions/dml/update.rs(1 hunks)src/engine/actions/etc.rs(1 hunks)src/engine/actions/mod.rs(1 hunks)src/engine/ast/ddl/alter_database.rs(1 hunks)src/engine/ast/ddl/alter_table.rs(1 hunks)src/engine/ast/ddl/create_database.rs(1 hunks)src/engine/ast/ddl/create_index.rs(2 hunks)src/engine/ast/ddl/create_table.rs(2 hunks)src/engine/ast/ddl/drop_database.rs(1 hunks)src/engine/ast/ddl/drop_table.rs(1 hunks)src/engine/ast/dml/delete.rs(2 hunks)src/engine/ast/dml/expressions/between.rs(2 hunks)src/engine/ast/dml/expressions/binary.rs(2 hunks)src/engine/ast/dml/expressions/call.rs(1 hunks)src/engine/ast/dml/expressions/list.rs(1 hunks)src/engine/ast/dml/expressions/not_between.rs(2 hunks)src/engine/ast/dml/expressions/parentheses.rs(1 hunks)src/engine/ast/dml/expressions/subquery.rs(1 hunks)src/engine/ast/dml/expressions/unary.rs(2 hunks)src/engine/ast/dml/insert.rs(2 hunks)src/engine/ast/dml/parts/from.rs(1 hunks)src/engine/ast/dml/parts/group_by.rs(1 hunks)src/engine/ast/dml/parts/having.rs(1 hunks)src/engine/ast/dml/parts/insert_values.rs(1 hunks)src/engine/ast/dml/parts/join.rs(1 hunks)src/engine/ast/dml/parts/order_by.rs(1 hunks)src/engine/ast/dml/parts/select_item.rs(1 hunks)src/engine/ast/dml/parts/target.rs(1 hunks)src/engine/ast/dml/parts/update_item.rs(1 hunks)src/engine/ast/dml/parts/where.rs(1 hunks)src/engine/ast/dml/plan/delete/delete_plan.rs(2 hunks)src/engine/ast/dml/plan/delete/from.rs(1 hunks)src/engine/ast/dml/plan/select/filter.rs(1 hunks)src/engine/ast/dml/plan/select/from.rs(1 hunks)src/engine/ast/dml/plan/select/join.rs(1 hunks)src/engine/ast/dml/plan/select/no_from.rs(1 hunks)src/engine/ast/dml/plan/select/scan.rs(1 hunks)src/engine/ast/dml/plan/select/select_plan.rs(2 hunks)src/engine/ast/dml/plan/select/subquery.rs(1 hunks)src/engine/ast/dml/plan/update/from.rs(1 hunks)src/engine/ast/dml/plan/update/update_plan.rs(2 hunks)src/engine/ast/dml/select.rs(2 hunks)src/engine/ast/dml/update.rs(2 hunks)src/engine/ast/mod.rs(1 hunks)src/engine/ast/other/desc_table.rs(1 hunks)src/engine/ast/other/show_databases.rs(1 hunks)src/engine/ast/other/show_tables.rs(1 hunks)src/engine/ast/other/use_database.rs(1 hunks)src/engine/ast/tcl/begin_transaction.rs(1 hunks)src/engine/ast/tcl/commit.rs(1 hunks)src/engine/ast/tcl/rollback.rs(1 hunks)src/engine/ast/types/column.rs(1 hunks)src/engine/ast/types/expression.rs(5 hunks)src/engine/ast/types/foreign_key.rs(1 hunks)src/engine/ast/types/select_column.rs(1 hunks)src/engine/ast/types/table.rs(1 hunks)src/engine/encoder/mod.rs(1 hunks)src/engine/expression.rs(6 hunks)src/engine/initialize.rs(16 hunks)src/engine/lexer/operator_token.rs(1 hunks)src/engine/lexer/test/comment.rs(1 hunks)src/engine/lexer/test/select.rs(1 hunks)src/engine/lexer/tokenizer.rs(1 hunks)src/engine/lexer/tokens.rs(2 hunks)src/engine/mod.rs(2 hunks)src/engine/optimizer/optimizer.rs(1 hunks)src/engine/parser/implements/common.rs(1 hunks)src/engine/parser/implements/ddl/database.rs(1 hunks)src/engine/parser/implements/ddl/table.rs(1 hunks)src/engine/parser/implements/ddl/top_level.rs(1 hunks)src/engine/parser/implements/debug.rs(1 hunks)src/engine/parser/implements/dml/delete.rs(1 hunks)src/engine/parser/implements/dml/expression.rs(1 hunks)src/engine/parser/implements/dml/insert.rs(1 hunks)src/engine/parser/implements/dml/select.rs(1 hunks)src/engine/parser/implements/dml/update.rs(1 hunks)src/engine/parser/implements/other/backslash_command.rs(1 hunks)src/engine/parser/implements/other/desc.rs(1 hunks)src/engine/parser/implements/other/show.rs(1 hunks)src/engine/parser/implements/other/use.rs(1 hunks)src/engine/parser/implements/tcl/begin.rs(1 hunks)src/engine/parser/implements/tcl/commit.rs(1 hunks)src/engine/parser/implements/tcl/rollback.rs(1 hunks)src/engine/parser/implements/utils.rs(1 hunks)src/engine/parser/parser.rs(1 hunks)src/engine/parser/test/alter_database.rs(1 hunks)src/engine/parser/test/alter_table.rs(1 hunks)src/engine/parser/test/common.rs(1 hunks)src/engine/parser/test/create_database.rs(1 hunks)src/engine/parser/test/create_table.rs(1 hunks)src/engine/parser/test/ddl.rs(1 hunks)src/engine/parser/test/delete.rs(1 hunks)
⛔ Files not processed due to max files limit (37)
- src/engine/parser/test/drop_database.rs
- src/engine/parser/test/drop_table.rs
- src/engine/parser/test/expressions.rs
- src/engine/parser/test/insert.rs
- src/engine/parser/test/other.rs
- src/engine/parser/test/select.rs
- src/engine/parser/test/tcl.rs
- src/engine/parser/test/update.rs
- src/engine/schema/database.rs
- src/engine/schema/mod.rs
- src/engine/schema/row.rs
- src/engine/schema/table.rs
- src/engine/server/channel.rs
- src/engine/server/mod.rs
- src/engine/types.rs
- src/engine/wal/endec/implements/bitcode.rs
- src/engine/wal/manager/builder.rs
- src/engine/wal/manager/mod.rs
- src/executor/README.md
- src/executor/common.rs
- src/executor/encoder/mod.rs
- src/executor/implements/other/desc_table.rs
- src/executor/implements/other/mod.rs
- src/executor/implements/other/show_databases.rs
- src/executor/implements/other/show_tables.rs
- src/executor/implements/other/use_database.rs
- src/executor/mod.rs
- src/executor/predule.rs
- src/executor/util.rs
- src/main.rs
- src/pgwire/connection/connection.rs
- src/pgwire/connection/prepared_statement.rs
- src/pgwire/engine/engine.rs
- src/pgwire/engine/rrdb.rs
- src/server/README.md
- src/server/mod.rs
- src/server/predule.rs
💤 Files with no reviewable changes (1)
- src/common/fs.rs
🔇 Additional comments (105)
src/engine/ast/dml/parts/having.rs (1)
1-1: 임포트 경로 업데이트가 모듈 재조직과 일치합니다.
SQLExpression임포트 경로가 새로운engine::ast::types네임스페이스로 올바르게 업데이트되었습니다. 이는 PR의 모듈 구조 재조직 목표와 완벽하게 부합합니다.src/engine/ast/dml/plan/select/no_from.rs (1)
1-1: LGTM! 모듈 경로 업데이트가 올바릅니다.프로젝트 레이아웃 재구성에 따라 import 경로가
crate::ast::predule에서crate::engine::ast::predule로 올바르게 업데이트되었습니다. PR의 목표인 AST 모듈을 engine 네임스페이스 하위로 이동하는 작업과 일치합니다.src/engine/ast/dml/expressions/subquery.rs (1)
1-1: LGTM!import 경로를
engine::네임스페이스로 이동한 변경사항이 프로젝트 리팩토링 목표와 일치하며, 올바르게 적용되었습니다.src/engine/ast/dml/plan/select/subquery.rs (1)
1-1: 임포트 경로 업데이트가 올바르게 적용되었습니다.프로젝트 구조 재편성에 따라 임포트 경로가
crate::ast::types에서crate::engine::ast::types로 정확하게 변경되었습니다. 변경 사항이 명확하고 PR의 목적과 일치합니다.src/common/mod.rs (1)
1-2: LGTM! 모듈 구조가 명확합니다.새로운
command와fs모듈을 명확하게 노출하여 프로젝트 레이아웃 개선 목표에 부합합니다.src/common/command.rs (2)
6-9: LGTM! 테스트 가능한 커맨드 추상화입니다.
mockall::automock속성을 사용하여 테스트에서 모킹이 가능한 깔끔한 trait 설계입니다. 단일 책임 원칙을 잘 따르고 있습니다.
11-17: LGTM! 효율적인 구현입니다.Zero-sized type인
RealCommandRunner를 사용하여 런타임 오버헤드 없이 추상화를 제공합니다.Command::output()을 올바르게 위임하고 있습니다.src/engine/ast/dml/expressions/call.rs (1)
1-1: 임포트 경로 업데이트 확인 완료
crate::ast::types에서crate::engine::ast::types로의 임포트 경로 변경이 프로젝트 레이아웃 재구성 목표와 일치합니다.src/engine/ast/dml/plan/select/scan.rs (1)
1-1: 임포트 경로 변경 적절함엔진 네임스페이스로의 일관된 마이그레이션입니다.
src/engine/ast/dml/plan/delete/from.rs (1)
1-1: 네임스페이스 재구성 확인AST 관련 타입들의 엔진 네임스페이스 이동이 올바르게 반영되었습니다.
src/engine/ast/ddl/alter_table.rs (1)
1-5: 임포트 경로 업데이트 완료엔진 네임스페이스로의 임포트 경로 변경이 적절합니다. Line 1의 주석 처리된 임포트는 이후 정리가 필요할 수 있습니다.
src/engine/ast/dml/parts/insert_values.rs (1)
1-1: 임포트 경로 변경 확인엔진 네임스페이스로의 일관된 마이그레이션입니다.
src/engine/ast/types/column.rs (1)
1-1: 임포트 경로 리팩토링 적절
DataType임포트 경로가 새로운 엔진 네임스페이스 구조에 맞게 업데이트되었습니다.src/engine/ast/tcl/commit.rs (1)
1-1: TCL 모듈 임포트 경로 업데이트 확인엔진 네임스페이스로의 경로 변경이 올바릅니다.
src/engine/ast/other/desc_table.rs (1)
1-1: 네임스페이스 마이그레이션 완료 확인됨검색 결과
src/test.rs:9에서만use crate::ast::가 발견되었으나, 이는 주석 처리된 코드입니다. 활성 코드에서는 이전 임포트 경로가 남아있지 않으므로 네임스페이스 마이그레이션이 일관되고 완벽하게 적용되었습니다.src/engine/ast/dml/parts/where.rs (1)
1-1: 네임스페이스 리팩토링 변경사항 확인됨.
SQLExpression임포트 경로가engine네임스페이스로 올바르게 업데이트되었습니다. 로직 변경이 없으며 프로젝트 구조 개선에 부합합니다.src/engine/ast/dml/parts/order_by.rs (1)
1-1: 네임스페이스 업데이트 승인.임포트 경로가
engine::ast네임스페이스로 일관되게 마이그레이션되었습니다.src/engine/ast/dml/parts/from.rs (1)
1-1: 임포트 경로 업데이트 확인.
SubqueryExpression과TableName의 임포트 경로가engine::ast네임스페이스로 정확하게 업데이트되었습니다.src/engine/optimizer/optimizer.rs (1)
1-14: 임포트 구조 개선 승인.그룹화된 임포트에서 명시적인 개별 임포트 문으로 변경되었으며, 모든 경로가
engine::ast네임스페이스로 일관되게 업데이트되었습니다. 이는 가독성을 향상시키고 옵티마이저 로직에는 영향을 주지 않습니다.src/engine/ast/ddl/create_table.rs (2)
1-4: DDL 타입 임포트 경로 업데이트 승인.
CreateTableQuery관련 모든 타입들의 임포트 경로가engine::ast네임스페이스로 올바르게 마이그레이션되었습니다. 퍼블릭 API 및 기능에는 변경이 없습니다.
80-80: 테스트 임포트 경로 업데이트 확인.테스트 모듈의
DataType임포트도engine::ast경로로 일관되게 업데이트되었습니다.src/engine/ast/types/table.rs (1)
3-3: 타입 임포트 네임스페이스 마이그레이션 승인.
FromClause및FromTarget의 임포트 경로가engine::ast네임스페이스로 정확하게 업데이트되었으며,TableName의 변환 로직은 그대로 유지됩니다.src/engine/parser/implements/dml/expression.rs (1)
1-14: 파서 임포트 네임스페이스 전면 마이그레이션 승인.표현식 파서의 모든 임포트 경로가
engine::ast,engine::lexer,engine::parser네임스페이스로 일관되게 업데이트되었습니다. 파싱 로직 및 알고리즘은 변경되지 않았으며, 네임스페이스 재구성만 반영되었습니다.src/engine/parser/implements/other/desc.rs (1)
1-5: 네임스페이스 리팩토링 승인 완료.코드 검사를 통해 다음을 확인했습니다:
src/engine/parser/implements/other/desc.rs의 모든 임포트가 올바르게 업데이트됨crate::engine::parser::predule패턴이 다른 모든 파서 구현 파일(14개 이상)과 일관됨parse_desc_query메서드가src/engine/parser/parser.rs에서 올바르게 호출되고 있음- 테스트 파일
src/engine/parser/test/other.rs도 새 네임스페이스 임포트를 사용 중- 깨진 임포트나 충돌하는 참조 없음
모든 변경사항이 일관되게 적용되었으며 컴파일 오류는 발생하지 않을 것으로 예상됩니다.
src/engine/actions/dml/scan.rs (1)
6-18: DBEngine 및 engine 네임스페이스로의 이전이 일관적으로 잘 정리되었습니다.
DBEngine의 메서드로 옮기면서TableName,StorageEncoder,TableDataRow,ExecuteError모두crate::engine::*네임스페이스를 사용하도록 맞춘 부분이 전체 엔진 구조 리팩터링 방향과 잘 맞습니다. 내부 로직 자체는 기존full_scan동작을 그대로 유지하는 것으로 보이며, 에러 처리와 디코딩 흐름도 자연스럽습니다.Also applies to: 32-85
src/engine/ast/types/expression.rs (1)
1-12: AST/스키마 타입을 engine 네임스페이스로 통합한 부분이 잘 정리되었습니다.
Binary/Unary/Between/NotBetween/Call/List/Parentheses/SubqueryExpression,WhereClause,TableDataFieldType모두crate::engine::...경로를 사용하도록 바뀌어서, AST/스키마 타입이 엔진 모듈 아래로 일관되게 모인 것이 좋습니다.From<TableDataFieldType> for SQLExpression구현도 기존 매핑(정수/실수/불리언/문자열/배열/NULL)을 그대로 유지하고 있어 동작 변경 없이 경로만 재배치된 것으로 보입니다. 테스트 모듈도 동일한 네임스페이스를 참조하도록 맞춰져 있어 후속 유지보수에 유리할 것 같습니다.Also applies to: 231-247, 258-274
src/engine/ast/dml/parts/update_item.rs (1)
1-7: UpdateItem이 engine AST 타입을 사용하도록 정리된 부분 좋습니다.
SQLExpression을crate::engine::ast::types에서 가져오도록 바꿔서, DML 파트 정의가 전반적인 엔진 네임스페이스 구조와 일관되게 맞춰졌습니다. 구조체 자체의 의미나 동작 변화는 없어서 리팩터링 목적에 잘 부합합니다.src/engine/ast/dml/expressions/list.rs (1)
1-20: ListExpression이 engine AST 네임스페이스와 잘 맞춰졌습니다.
SQLExpressionimport 경로만crate::engine::ast::types로 이동했고,ListExpression↔SQLExpression::List간 변환 로직은 그대로 유지되어 있습니다. 리스트 표현식 관련 기능에는 영향을 주지 않으면서 모듈 구조만 깔끔하게 정리된 것으로 보입니다.src/engine/ast/ddl/drop_table.rs (1)
1-32: DropTableQuery가 engine AST 네임스페이스를 사용하도록 일관되게 정리되었습니다.
TableName,DDLStatement,SQLStatement를 모두crate::engine::ast에서 가져오도록 바뀌어 DDL 쿼리 타입이 엔진 모듈 아래로 자연스럽게 편입되었습니다.builder/set_table/set_if_exists/build체인 API는 변경되지 않아 기존 사용 코드는 그대로 동작할 것으로 보입니다.src/engine/ast/types/foreign_key.rs (1)
1-11: ForeignKey가 engine AST의 TableName을 참조하도록 정리된 점 좋습니다.외래키 정의에서 사용하는
TableName이crate::engine::ast::types로 이동해, 다른 AST 타입들과 동일한 네임스페이스를 공유하게 된 점이 구조적으로 자연스럽습니다. 필드 구성과 직렬화/비교 동작에는 변화가 없어 보입니다.src/engine/ast/tcl/begin_transaction.rs (1)
1-10: BeginTransactionQuery가 engine AST 네임스페이스로 편입된 부분이 자연스럽습니다.
SQLStatement,TCLStatement를crate::engine::ast에서 가져오도록 바뀌어 TCL 쿼리도 다른 AST들과 동일한 계층 구조를 따르게 되었습니다.From<BeginTransactionQuery> for SQLStatement구현은 그대로라 기존 트랜잭션 시작 흐름에 영향은 없을 것으로 보입니다.src/engine/ast/types/select_column.rs (1)
1-25: SelectColumn이 engine AST 타입으로 정리되어 일관성이 좋아졌습니다.
SQLExpression을crate::engine::ast::types에서 가져오도록 변경해,SelectColumn→SQLExpression::SelectColumn변환이 엔진 AST 타입 체계 안에서 완결되도록 한 점이 좋습니다. 구조체와 생성자(new) 자체는 바뀐 부분이 없어 기존 SELECT 컬럼 처리 동작도 그대로 유지될 것으로 보입니다.src/engine/lexer/tokenizer.rs (1)
3-3: 임포트 경로 업데이트 확인 완료네임스페이스 재구성에 따라
crate::lexer::predule에서crate::engine::lexer::predule로 임포트 경로가 올바르게 업데이트되었습니다.src/engine/ast/dml/plan/select/filter.rs (1)
1-1: 임포트 경로 변경 승인
SQLExpression타입의 임포트 경로가engine::ast네임스페이스로 일관되게 업데이트되었습니다.src/engine/ast/other/show_databases.rs (1)
1-1: 네임스페이스 이동 확인AST 관련 타입들이
engine::ast네임스페이스로 올바르게 이동되었습니다.src/engine/ast/dml/parts/group_by.rs (1)
1-1: 임포트 경로 업데이트 승인
SelectColumn타입의 임포트 경로가 리팩토링 방향에 맞게 업데이트되었습니다.src/engine/ast/dml/plan/update/from.rs (1)
1-1: 임포트 경로 정리 확인DML 플랜 관련 타입들이
engine::ast네임스페이스로 일관되게 이동되었습니다.src/engine/ast/dml/parts/select_item.rs (1)
1-1: 임포트 변경 승인
SQLExpression임포트가 엔진 네임스페이스로 올바르게 업데이트되었습니다.src/engine/ast/dml/expressions/between.rs (1)
3-3: 메인 코드 및 테스트 임포트 일관성 확인
SQLExpression임포트가 메인 코드와 테스트 코드 모두에서engine::ast::types로 일관되게 업데이트되었습니다.Also applies to: 49-49
src/engine/parser/test/ddl.rs (1)
3-12: 테스트 파일 임포트 경로 업데이트 완료DDL 테스트 파일의 모든 임포트가
engine::네임스페이스로 체계적으로 업데이트되었습니다. 테스트 로직은 변경되지 않았으며, 임포트 경로만 리팩토링 방향에 맞게 수정되었습니다.src/engine/ast/dml/plan/select/from.rs (1)
1-1: 임포트 경로 업데이트가 올바르게 적용되었습니다.
crate::ast에서crate::engine::ast로의 네임스페이스 마이그레이션이 정확합니다.src/engine/ast/mod.rs (1)
9-21: 자체 참조 임포트가 올바르게 구성되었습니다.
engine::ast모듈에서 자신의 하위 모듈들을crate::engine::ast로 임포트하는 것은 새로운 구조에서 올바른 접근입니다.src/engine/ast/other/show_tables.rs (1)
1-1: 임포트 경로가 일관되게 업데이트되었습니다.
OtherStatement와SQLStatement의 임포트 경로가 새로운engine::ast네임스페이스로 올바르게 변경되었습니다.src/engine/ast/ddl/alter_database.rs (1)
1-1: DDL 관련 임포트가 정확하게 업데이트되었습니다.
DDLStatement와SQLStatement의 경로가engine::ast로 올바르게 마이그레이션되었습니다.src/engine/ast/dml/insert.rs (1)
1-1: 프로덕션 코드와 테스트 코드의 임포트가 일관되게 업데이트되었습니다.메인 코드(Line 1)와 테스트 코드(Line 68) 모두에서
engine::ast경로로의 변경이 일관성 있게 적용되었습니다.Also applies to: 68-68
src/engine/ast/other/use_database.rs (1)
1-1: 임포트 경로 변경이 적절합니다.
OtherStatement와SQLStatement의 임포트가 새로운 모듈 구조에 맞게 업데이트되었습니다.src/engine/ast/dml/delete.rs (1)
1-1: DML 삭제 쿼리의 임포트가 체계적으로 업데이트되었습니다.프로덕션 코드와 테스트 코드의 임포트가 모두
engine::ast네임스페이스로 일관되게 마이그레이션되었습니다.Also applies to: 53-53
src/config/mod.rs (1)
1-1: 리팩토링 완료 확인 - 모든GlobalConfig참조가 성공적으로LaunchConfig로 업데이트되었습니다.검증 결과:
GlobalConfig참조: 0건 (전체 제거됨)LaunchConfig정의: src/config/launch_config.rs:11에서 확인launch_config모듈 사용: 8개 파일에서 올바르게 import됨코드베이스 전체에서 리팩토링이 완전히 진행되었습니다.
src/engine/ast/tcl/rollback.rs (1)
1-1: engine 네임스페이스로 AST import 경로를 옮긴 부분 문제 없어 보입니다.동작을 바꾸지 않고
RollbackQuery -> SQLStatement변환 로직은 그대로 유지한 채 경로만crate::engine::ast로 맞춰서, 전체 엔진 레이아웃 리팩토링과 잘 정합되는 변경입니다.src/engine/ast/dml/expressions/parentheses.rs (1)
1-1: SQLExpression import를 engine AST 타입 네임스페이스로 이동한 부분 적절합니다.
ParenthesesExpression구조와From<ParenthesesExpression> for SQLExpression구현은 그대로이고, 참조 경로만 새crate::engine::ast::types로 정리되어 리팩토링 의도에 잘 맞습니다.src/engine/ast/dml/parts/target.rs (1)
1-1: UpdateTarget에서 TableName import 경로 변경이 일관성 있게 적용되었습니다.
UpdateTarget와From<TableName> for UpdateTarget의 동작은 유지하면서,TableName을crate::engine::ast::types에서 가져오도록만 수정되어 엔진 하위 AST 타입 구조와 잘 맞습니다.src/engine/ast/dml/expressions/not_between.rs (1)
1-1: NotBetweenExpression 본문과 테스트 모두 engine AST 타입 네임스페이스로 정리된 점 좋습니다.프로덕션 코드와 테스트 모두
SQLExpression을crate::engine::ast::types에서 가져오도록 통일해, 네임스페이스 리팩토링 이후에도 타입 일관성이 유지됩니다. 표현식 타입과From구현은 변함없어 회귀 위험도 낮아 보입니다.Also applies to: 29-29
src/engine/lexer/test/comment.rs (1)
2-2: lexer 코멘트 토큰 테스트가 engine 네임스페이스를 사용하도록 잘 정리되었습니다.
Token과Tokenizer를crate::engine::lexer::predule에서 가져오도록만 바뀌었고, 실제 토큰 시퀀스와 기대값(assert) 로직은 그대로라 리팩토링 후에도 동일한 동작을 검증합니다.src/engine/parser/implements/utils.rs (1)
1-1: Parser 유틸에서 Parser/Token import를 engine 모듈로 옮긴 변경이 깔끔합니다.
Parser확장 메서드들의 시그니처와 구현은 그대로 유지하면서, 의존 경로만crate::engine::parser::predule및crate::engine::lexer::predule로 옮겨져 새 엔진 중심 구조와 잘 맞습니다.Also applies to: 3-3
src/engine/parser/test/delete.rs (1)
3-10: DELETE 파서 테스트의 AST/lexer/parser import들을 engine 네임스페이스로 일괄 변경한 부분이 잘 정리되었습니다.
DeleteQuery, 표현식 관련 타입들, 토큰,Parser모두를crate::engine::...경로로 가져오도록 통일해, 새 엔진 모듈 구조에 맞추면서 테스트 로직과 기대값 구성은 그대로 유지되고 있습니다.src/engine/ast/dml/parts/join.rs (1)
1-1: JoinClause에서 사용하는 AST 타입 import를 engine::ast::types로 옮긴 변경이 적절합니다.
JoinClause와JoinType정의는 그대로이고,SQLExpression,TableName참조 경로만 새 엔진 AST 타입 모듈로 이동해, 직렬화/파생 설정과도 충돌 없이 구조 리팩토링 목적에 부합합니다.src/engine/ast/dml/select.rs (1)
3-7: LGTM! 임포트 경로 업데이트가 올바르게 적용되었습니다.
crate::ast에서crate::engine::ast로의 임포트 경로 변경이 일관되게 적용되었으며, 테스트 모듈의 임포트도 함께 업데이트되었습니다.Also applies to: 233-236
src/engine/parser/test/create_database.rs (1)
2-5: LGTM! 테스트 임포트 경로가 올바르게 업데이트되었습니다.모든 테스트 의존성이
engine::네임스페이스로 일관되게 마이그레이션되었습니다.src/engine/parser/test/common.rs (1)
2-9: LGTM! 공통 테스트 유틸리티 임포트가 올바르게 업데이트되었습니다.AST, lexer, parser 모듈에 대한 모든 임포트가
engine::네임스페이스로 일관되게 변경되었습니다.src/engine/parser/implements/other/show.rs (1)
1-3: LGTM! Parser 구현 임포트가 올바르게 업데이트되었습니다.SHOW 쿼리 파서의 모든 의존성이
engine::네임스페이스로 정확하게 마이그레이션되었습니다.Also applies to: 6-7
src/engine/parser/implements/dml/delete.rs (1)
1-1: LGTM! DELETE 파서 임포트가 올바르게 업데이트되었습니다.DML DELETE 쿼리 파서의 모든 임포트가
engine::네임스페이스로 정확하게 변경되었습니다.Also applies to: 3-3, 6-6
src/engine/ast/dml/plan/delete/delete_plan.rs (1)
1-1: LGTM! DELETE 플랜 임포트가 올바르게 업데이트되었습니다.FilterPlan 임포트와 테스트 모듈의 임포트가 모두
engine::ast경로로 일관되게 변경되었습니다.Also applies to: 31-34
src/engine/ast/dml/plan/select/join.rs (1)
1-4: LGTM! JOIN 플랜 임포트가 올바르게 업데이트되었습니다.조인 관련 타입들의 임포트가
engine::ast네임스페이스로 정확하게 마이그레이션되었습니다.src/engine/ast/ddl/create_index.rs (1)
1-4: CREATE INDEX 임포트 경로 마이그레이션 완료 및 검증됨검증 결과
src/engine/ast/ddl/create_index.rs의 모든 임포트가crate::engine::ast::네임스페이스로 올바르게 업데이트되었습니다. 프로젝트 전체에서 332개의 활성engine::임포트문이 일관되게 적용되었으며, 기존의 낡은 스타일 임포트는 찾아지지 않았습니다(src/test.rs의 주석 처리된 코드 제외). 메인 코드와 테스트 코드 모두 새로운 네임스페이스를 사용하고 있어 마이그레이션이 완전히 이루어졌습니다.src/engine/parser/implements/tcl/commit.rs (1)
1-4: LGTM! 네임스페이스 마이그레이션이 정확합니다.
crate::ast에서crate::engine::ast로,crate::parser에서crate::engine::parser로의 임포트 경로 업데이트가 올바르게 적용되었습니다. 로직 변경 없이 구조적 리팩토링만 수행되었습니다.src/engine/encoder/mod.rs (1)
1-1: LGTM! 모듈 노출이 적절합니다.
schema_encoder모듈을 공개적으로 노출하여engine::encoder네임스페이스 하에서 접근 가능하게 했습니다.src/engine/parser/implements/other/backslash_command.rs (1)
1-6: LGTM! 임포트 경로가 올바르게 업데이트되었습니다.AST, 렉서, 파서 모듈의 임포트 경로가 모두
engine::네임스페이스로 일관되게 마이그레이션되었습니다.src/engine/parser/implements/tcl/rollback.rs (1)
1-4: LGTM! 네임스페이스 마이그레이션이 정확합니다.TCL 관련 AST 및 파서 임포트가
engine::네임스페이스로 올바르게 업데이트되었습니다.src/engine/lexer/test/select.rs (1)
2-4: LGTM! 테스트 임포트가 올바르게 업데이트되었습니다.렉서 테스트의 임포트 경로가
engine::lexer::predule로 일관되게 업데이트되었으며, 테스트 로직에는 변경사항이 없습니다.src/engine/parser/implements/common.rs (1)
1-7: LGTM! 모든 임포트 경로가 일관되게 업데이트되었습니다.AST 타입, 렉서, 파서 모듈의 임포트가 모두
engine::네임스페이스로 올바르게 마이그레이션되었습니다. 파싱 로직에는 변경사항이 없습니다.src/engine/parser/implements/ddl/database.rs (1)
1-9: LGTM! DDL 파서 임포트가 올바르게 업데이트되었습니다.데이터베이스 DDL 관련 AST, 렉서, 파서 임포트 경로가 모두
engine::네임스페이스로 일관되게 마이그레이션되었습니다.src/engine/actions/mod.rs (1)
3-3: 모듈 이름 변경 확인 완료 - 모든 참조가 올바르게 업데이트됨검증 결과 모듈 이름 변경이 완전하게 처리되었습니다:
- ✓
pub mod etc;- src/engine/actions/mod.rs의 3번 라인에서 새 모듈명 적용됨- ✓
actions::other참조 - 코드베이스 전체에서 발견되지 않음- ✓ 이전
other디렉토리 - 제거됨- ✓ 새
etc.rs모듈 파일 - 정상 위치에 존재src/engine/ast/dml/update.rs (2)
1-1: AST 임포트 경로를 engine 네임스페이스로 옮긴 부분 확인했습니다
TableName,DMLStatement,SQLStatement모두crate::engine::ast아래로 정리되어, PR 목적(엔진 중심 레이아웃)과 잘 맞고, 타입 일관성도 유지됩니다.
57-76: 테스트에서 SQLExpression 경로 정렬이 잘 되어 있습니다테스트 모듈도
crate::engine::ast::types::SQLExpression을 사용하도록 맞춰서, 프로덕션 코드와 네임스페이스가 일치합니다. 리팩토링 이후에도 테스트가 같은 타입을 바라보도록 잘 조정된 것 같습니다.src/engine/ast/dml/plan/select/select_plan.rs (2)
1-1: Group/Order 절 관련 임포트 경로 변경이 일관적입니다
GroupByClause,OrderByClause를crate::engine::ast::dml::parts에서 가져오도록 바꾼 것은, 다른 DML AST 모듈과도 정합성이 있어서 구조적으로 자연스럽습니다.
70-76: SelectPlan 테스트 임포트가 새 엔진 구조와 잘 맞습니다테스트에서 사용하는
JoinType,SelectFromPlan,JoinScanType,ScanType,SQLExpression,TableName모두crate::engine::ast트리로 이동하여, 실제 모듈 구조와 어긋나는 부분이 없습니다. 리네임/이동에 따른 테스트 정리도 잘 되어 있습니다.src/engine/parser/parser.rs (1)
3-6: 파서 상단 의존성을 engine 네임스페이스로 모은 방향이 적절합니다
SQLStatement,Token/Tokenizer,ParserContext를 모두crate::engine아래에서 가져오도록 통일해 둔 점이 좋습니다. 파서 구현부에는 추가 변경이 없어, 동작을 건드리지 않고 레이아웃만 재구성한 것으로 보입니다.src/engine/ast/dml/expressions/unary.rs (2)
1-1: Unary 표현식에서 사용하는 SQLExpression 임포트 변경 확인했습니다
SQLExpression을crate::engine::ast::types에서 가져오도록 변경해 엔진 쪽 AST 트리에 자연스럽게 합류시켰습니다. 타입 사용처와도 일관적입니다.
27-30: UnaryOperator 테스트 임포트도 새 AST 위치와 잘 맞습니다테스트에서
UnaryOperator와SQLExpression을 각각crate::engine::ast::dml::expressions::operators및crate::engine::ast::types에서 임포트하도록 바꾼 부분이 실제 구조와 정합적으로 보입니다. 리팩토링 후에도 테스트가 동일 타입을 참조하도록 잘 정리되어 있습니다.src/engine/ast/ddl/drop_database.rs (1)
1-1: DDL re-export 경로를 engine::ast 로 옮긴 부분이 자연스럽습니다
DropDatabaseQuery::build가 여전히SQLStatement::DDL(DDLStatement::DropDatabaseQuery(...))를 생성하는 구조와 잘 맞고, 이제 이 파일이 엔진 쪽 AST 타입을 re-export 하는 관문 역할을 명확히 합니다.src/engine/parser/test/alter_table.rs (1)
3-9: AST/파서 테스트 import 경로 리팩토링은 문제 없어 보입니다.
AlterTable*,Column/DataType/SQLExpression/TableName,Parser/ParserContext를 모두crate::engine::...아래에서 가져오도록 바꾼 부분이 현재 엔진 모듈 구조와 잘 맞고, 테스트 본문 로직은 그대로라 동작 변화는 없을 것으로 보입니다.src/engine/parser/test/alter_database.rs (1)
2-7: 엔진 네임스페이스로의 테스트 import 이관이 잘 정리되었습니다.
AlterDatabase*,SQLStatement,Token,Parser를crate::engine::...아래로 옮긴 부분이 다른 테스트들과 일관적이고, 테스트 케이스 구성 및 검증 로직은 그대로 유지되어 있어 기능 변화 없이 구조만 정리된 것으로 보입니다.src/engine/lexer/operator_token.rs (1)
1-3: 연산자/에러 타입 경로 변경은 안전해 보입니다.
BinaryOperator/UnaryOperator와IntoError/RRDBError를crate::engine::...경로로 옮긴 것은 전체 AST/에러 모듈 리팩토링 방향과 잘 맞고, 아래TryInto구현이나 테스트 코드에는 영향이 없습니다. 이 파일은 경로 재배치만 이뤄진 것으로 판단됩니다.src/engine/ast/dml/expressions/binary.rs (1)
1-1: Binary 연산식 AST의 엔진 경로 이관이 잘 되어 있습니다.
SQLExpression을crate::engine::ast::types로, 테스트 쪽BinaryOperatorExpression/SQLExpressionimport 를crate::engine::ast::dml::...경로로 옮긴 부분이 전체 AST 재구성 방향과 일관적입니다. From 구현 및 테스트의 기대값 구성은 그대로 유지되어 있어서, 동작에는 변화 없이 네임스페이스만 정리된 것으로 보입니다.Also applies to: 46-52
src/engine/parser/implements/dml/select.rs (1)
3-13: SELECT 파서의 엔진 AST/파서/렉서 경로 이관은 적절합니다.
WhereClause,GroupByItem,HavingClause,JoinClause/JoinType,OrderBy*,SelectItem/SelectWildCard,SelectQuery와OperatorToken/Token,Parser/ParserContext를 모두crate::engine::...경로로 옮긴 부분이 DML 쿼리 관련 모듈들을 엔진 네임스페이스 아래로 모으는 목적과 잘 맞습니다. 구현부는 그대로여서 기능적인 변화 없이 레이아웃만 정리된 것으로 보입니다.src/engine/parser/implements/dml/insert.rs (1)
1-6: LGTM! 네임스페이스 재구성이 일관되게 적용되었습니다.import 경로가
crate::engine::네임스페이스로 올바르게 업데이트되었으며, 로직 변경은 없습니다.src/engine/lexer/tokens.rs (2)
2-4: LGTM! Import 경로가 올바르게 업데이트되었습니다.
BinaryOperator와 관련 타입들의 경로가crate::engine::ast네임스페이스로 일관되게 변경되었습니다.
189-192: LGTM! 테스트 import도 일관되게 업데이트되었습니다.테스트 모듈의 import 경로가
engine::네임스페이스를 올바르게 사용하고 있습니다.src/engine/ast/dml/plan/update/update_plan.rs (2)
1-1: LGTM! FilterPlan import 경로가 올바르게 업데이트되었습니다.네임스페이스 변경이 정확하게 반영되었습니다.
31-34: LGTM! 테스트 import도 일관성 있게 변경되었습니다.테스트 코드의 import 경로가
engine::네임스페이스를 올바르게 사용합니다.src/engine/ast/ddl/create_database.rs (1)
1-1: LGTM! Public API 경로가 올바르게 재구성되었습니다.
DDLStatement와SQLStatement가crate::engine::ast네임스페이스를 통해 올바르게 노출되고 있습니다.src/config/launch_config.rs (2)
11-11: LGTM! GlobalConfig에서 LaunchConfig로의 리네이밍이 명확합니다.새로운 이름이 설정의 용도(launch configuration)를 더 명확하게 표현합니다.
53-63: LGTM! 설정 파일 로딩 기능이 적절히 추가되었습니다.
load_from_path메서드가 TOML 파일로부터 설정을 로드하는 기능을 제공하며, 기본 경로 사용을 지원합니다.src/engine/parser/test/create_table.rs (1)
3-6: LGTM! 테스트 파일의 import가 일관되게 업데이트되었습니다.모든 import 경로가
engine::네임스페이스를 올바르게 사용하고 있으며, 테스트 로직은 변경되지 않았습니다.src/engine/parser/implements/dml/update.rs (1)
1-7: LGTM! Parser import가 일관되게 업데이트되었습니다.DML update 관련 타입들의 import 경로가
engine::네임스페이스로 올바르게 변경되었으며, 파싱 로직은 변경되지 않았습니다.src/engine/expression.rs (3)
1-15: LGTM! Import 경로가 일관되게 업데이트되었습니다.모든 타입과 모듈이
engine::네임스페이스로 올바르게 이동되었습니다.
25-26: LGTM! Executor에서 DBEngine으로의 전환과 가시성 축소가 적절합니다.
impl DBEngine변경은 PR의 리팩토링 목표에 부합하며,reduce_expression의 가시성을pub(crate)로 제한하는 것은 API surface area를 줄이는 좋은 개선입니다.
389-396: LGTM! 로직 변경 없이 포맷팅만 조정되었습니다.이진 연산자 처리 로직은 동일하게 유지되고 있습니다.
src/engine/actions/dml/select.rs (2)
6-21: DBEngine 및 engine 네임스페이스 기반으로의 리팩터링 일관성이 좋습니다.
DBEngine을 대상으로 하는impl과crate::engine::...경로로의 import 정리가 전체 엔진 레이아웃 개편 방향과 잘 맞고, 함수 시그니처/반환 타입(ExecuteResult,RRDBError)도 기존 실행 흐름과 자연스럽게 이어집니다.
120-133: 리뷰 코멘트는 부분적으로만 정확합니다. 컴파일 에러는 발생하지 않지만, 논리적 오류는 존재합니다.코드 검증 결과:
문법 및 타입 검증:
if let Some(table_name) = table_alias_reverse_map.get(table_name) && table_name == &field.table_name.table_name구문은 Rust 2021+ 표준 문법이며 타입도 정상입니다.table_alias_reverse_map은HashMap<String, String>타입이고,.get()반환값Option<&String>과 비교&String == &String은 모두 타입이 맞습니다.- 따라서 컴파일 에러는 발생하지 않습니다.
논리적 오류:
- 51줄:
table_alias_reverse_map.insert(table_name.clone().table_name, alias);로 맵을 생성할 때, 키는 실제 테이블명, 값은 별칭입니다.- 128-130줄: 이 맵의 값(별칭)과
field.table_name.table_name(테이블명)을 비교하고 있어, 실제로는 절대 일치할 수 없습니다.- 맵의 키-값 관계가 의도와 맞지 않습니다.
권장사항:
제안하신 리팩토링은 정확합니다. 중첩된if로 분리하고 변수명을 명확히 하면 가독성과 유지보수성이 개선됩니다:} else if let Some(alias) = table_alias_reverse_map.get(table_name) { if alias == &field.table_name.table_name { table_name_matched = true; } }Likely an incorrect or invalid review comment.
src/engine/parser/implements/ddl/top_level.rs (1)
1-6: 파서 관련 타입을 engine 네임스페이스로 옮긴 변경이 구조적으로 자연스럽습니다.
SQLStatement,Token,ParserContext,Parser를 모두crate::engine::...경로로 모은 덕분에 파싱 계층이 엔진 하위로 명확히 묶여서 이후 모듈 구조 이해에 도움이 될 것 같습니다.src/engine/actions/ddl/create_database.rs (1)
3-91: DBEngine/engine 네임스페이스 기반으로의create_database리팩터링이 안정적으로 보입니다.
CreateDatabaseQuery,StorageEncoder,DatabaseSchema,Execute*타입을 모두crate::engine::...으로 모으고impl DBEngine에서 데이터 디렉터리 경로 계산 → DB 디렉터리/tables서브디렉터리 생성 →database.config에DatabaseSchema를 encode 해서 기록하는 흐름이 자연스럽게 이어집니다.기존 동작을 크게 바꾸지 않으면서 타입과 레이아웃만 정리된 형태라, 아키텍처 측면에서 무난하게 잘 정리된 것 같습니다.
src/engine/actions/ddl/alter_database.rs (1)
3-10: DBEngine 및 engine 스키마/인코더 타입으로의 전환이 create_database 와 잘 대칭을 이룹니다.
AlterDatabaseQuery/Action,StorageEncoder,DatabaseSchema,Execute*타입을 모두crate::engine::...하위로 가져오고impl DBEngine에서 처리하게 만든 구조가create_database와도 자연스럽게 짝을 이루어서, DDL 액션들이 엔진 레이어에 일관되게 모인 느낌입니다.Also applies to: 14-18
src/engine/initialize.rs (1)
16-23:DBEngine::initialize로 초기화 플로우를 묶은 설계는 좋습니다.
init_config→init_database순으로 한 번만 공개 API에서 호출하게 한 점이 명확하고, 외부에서는DBEngine::initialize().await만 알면 되어서 사용성이 좋아졌습니다. 나머지 헬퍼들을 비공개 메서드로 숨긴 것도 방향성이 괜찮습니다.src/engine/mod.rs (1)
1-15: 엔진 모듈 네임스페이스 정리가 명확합니다
engine아래로 ast/encoder/lexer/parser/schema/server/wal 및 DB 엔진 전용 actions/expression/initialize/types를 정리한 구조가 역할별로 잘 나뉘어 있어서, 이후 유지보수와 탐색성이 좋아질 것 같습니다. 이 방향성은 전체 PR 목표(레이아웃 재구성)에 잘 부합합니다.
| let futures = read_dir_result.map(|e| async { | ||
| match e { | ||
| Ok(entry) => match entry.file_type() { | ||
| Ok(file_type) => { | ||
| if file_type.is_dir() { | ||
| let mut path = entry.path(); | ||
| path.push("database.config"); | ||
|
|
||
| match tokio::fs::read(path).await { | ||
| Ok(result) => { | ||
| let database_config: DatabaseSchema = | ||
| encoder.decode(result.as_slice()).unwrap(); | ||
|
|
||
| Some(database_config.database_name) | ||
| } | ||
| Err(_) => None, | ||
| } | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
show_databases에서 encoder.decode(...).unwrap()은 설정 파일 손상 시 패닉을 유발합니다
match tokio::fs::read(path).await {
Ok(result) => {
let database_config: DatabaseSchema =
encoder.decode(result.as_slice()).unwrap();
Some(database_config.database_name)
}
Err(_) => None,
}여기서 decode가 None을 반환하면 unwrap()이 패닉을 일으켜 엔진 전체가 종료됩니다. 같은 파일의 다른 코드에서는 디코드 실패를 안전하게 처리하고 있어(예: desc_table의 ok_or_else, show_tables의 match encoder.decode(...) { Some(decoded) => ..., None => return None }), 이곳도 일관되게 방어적으로 처리하는 것이 좋아 보입니다.
예시:
- match tokio::fs::read(path).await {
- Ok(result) => {
- let database_config: DatabaseSchema =
- encoder.decode(result.as_slice()).unwrap();
-
- Some(database_config.database_name)
- }
- Err(_) => None,
- }
+ match tokio::fs::read(path).await {
+ Ok(result) => {
+ let database_config: Option<DatabaseSchema> =
+ encoder.decode(result.as_slice());
+
+ match database_config {
+ Some(config) => Some(config.database_name),
+ None => None, // 부정확한 config는 조용히 스킵
+ }
+ }
+ Err(_) => None,
+ }이렇게 하면 잘못된 database.config 하나 때문에 전체 쿼리 처리가 중단되는 일을 피할 수 있습니다.
I need to verify this review comment by examining the actual code. Let me search the repository to confirm the issue.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the specific file and lines to verify the unwrap() usage
cat "src/engine/actions/etc.rs" | sed -n '90,130p'
Length of output: 1687
🏁 Script executed:
#!/bin/bash
# Search for encoder.decode patterns in the file to verify inconsistent handling
rg -n "encoder\.decode" "src/engine/actions/etc.rs" -C 5Length of output: 1317
🏁 Script executed:
#!/bin/bash
# Search for the functions mentioned: desc_table and show_tables
rg -n "fn desc_table|fn show_tables" "src/engine/actions/etc.rs" -A 15Length of output: 1540
더 자세히 desc_table 함수의 전체 구현을 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full desc_table function implementation to see error handling
rg -n "fn desc_table" "src/engine/actions/etc.rs" -A 40
Length of output: 1878
🏁 Script executed:
#!/bin/bash
# Search for all ok_or_else patterns mentioned in review to find error handling examples
rg -n "ok_or_else|match encoder\.decode" "src/engine/actions/etc.rs" -B 2 -A 2Length of output: 632
show_databases의 설정 파일 디코드 시 unwrap() 사용으로 인한 패닉 위험 해결 필요
검증 결과 리뷰 의견의 지적이 정확합니다. 같은 파일 내에서 encoder.decode() 호출 시 다음과 같은 일관되지 않은 오류 처리 패턴이 확인됩니다:
desc_table(36-38줄):.ok_or_else(|| ExecuteError::wrap("config decode error"))?;- 안전한 에러 전파show_tables(182-184줄):match encoder.decode(result.as_slice()) { Some(decoded) => decoded, None => return None }- 안전한 처리show_databases(111줄):encoder.decode(result.as_slice()).unwrap()- 패닉 유발
111줄의 unwrap()은 손상된 database.config 파일 하나로 인해 전체 엔진을 중단시킵니다. 제안된 수정안(match 패턴 사용 또는 ok_or_else)을 적용하여 일관된 오류 처리로 변경하세요.
🤖 Prompt for AI Agents
In src/engine/actions/etc.rs around lines 100 to 117, the code uses
encoder.decode(...).unwrap() which can panic if the config is corrupted; replace
the unwrap with safe error handling consistent with the rest of the module
(e.g., match encoder.decode(result.as_slice()) { Some(decoded) => decoded, None
=> return None } or map the error to an ExecuteError via ok_or_else and
propagate), so a malformed database.config yields a controlled None/error
instead of crashing the process.
| _ => { | ||
| return Err(TypeError::wrap( | ||
| "unary '!' operator is valid only for integer and float types.", | ||
| )); | ||
| } |
There was a problem hiding this comment.
에러 메시지가 부정확합니다.
UnaryOperator::Neg 브랜치에서 배열 요소 처리 중 발생하는 에러 메시지가 "unary '!' operator"라고 표시되어 있지만, 실제로는 "unary '-' operator"여야 합니다. Line 83-85의 에러 메시지가 올바른 표현입니다.
_ => {
return Err(TypeError::wrap(
- "unary '!' operator is valid only for integer and float types.",
+ "unary '-' operator is valid only for integer and float types.",
));
}🤖 Prompt for AI Agents
In src/engine/expression.rs around lines 74 to 78, the error message in the
fallback arm incorrectly references the unary '!' operator while this branch is
for UnaryOperator::Neg; update the TypeError::wrap string to mention the unary
'-' operator (matching the message used at lines 83-85) so the error correctly
reads that the unary '-' operator is valid only for integer and float types.
| let test_cases = vec![ | ||
| TestCase { | ||
| name: "init 정상 동작 (linux)", | ||
| want_error: false, | ||
| mock_config: Box::new(|| { | ||
| let config = GlobalConfig::default(); | ||
| let config = LaunchConfig::default(); | ||
|
|
There was a problem hiding this comment.
테스트에서 test_cases[..5]만 순회하여 마지막 두 케이스가 실행되지 않습니다.
test_init_config에서:
for test_case in test_cases[..5].iter() {
...
}로 슬라이스를 잘라 사용하고 있어서, 정의된 7개의 TestCase 중 앞의 5개만 실행되고 마지막 두 케이스(“전역 설정파일 생성 실패”, “최상위 디렉터리 생성 실패”)는 아예 검증되지 않습니다.
전체 케이스를 모두 돌리려면 단순히:
- for test_case in test_cases[..5].iter() {
+ for test_case in &test_cases {처럼 변경하는 편이 의도에도 더 잘 맞고, 새로 추가된 실패 시나리오에 대한 회귀도 막을 수 있습니다.
Also applies to: 313-314, 370-371, 420-421, 480-481, 522-523, 556-557, 579-597
🤖 Prompt for AI Agents
In src/engine/initialize.rs around lines 241-247 (and similarly at 313-314,
370-371, 420-421, 480-481, 522-523, 556-557, 579-597), the tests iterate over a
sliced subset of test_cases (e.g., test_cases[..5]) which skips the last two
defined cases; change the iteration to cover the entire vector (e.g., iterate
over test_cases.iter()) so all defined TestCase entries are executed and
validated, updating each occurrence noted to remove the hardcoded slice bounds.
| pub async fn get_table_config(&self, table_name: TableName) -> Result<TableSchema, RRDBError> { | ||
| let encoder = StorageEncoder::new(); | ||
|
|
||
| let base_path = self.get_data_directory(); | ||
|
|
||
| let TableName { | ||
| database_name, | ||
| table_name, | ||
| } = table_name; | ||
|
|
||
| let database_name = database_name.unwrap(); | ||
|
|
||
| let database_path = base_path.clone().join(&database_name); | ||
| let table_path = database_path.clone().join("tables").join(&table_name); | ||
|
|
||
| // config data 파일 내용 변경 | ||
| let config_path = table_path.clone().join("table.config"); | ||
|
|
||
| match tokio::fs::read(&config_path).await { | ||
| Ok(data) => { | ||
| let table_config: Option<TableSchema> = encoder.decode(data.as_slice()); | ||
|
|
||
| match table_config { | ||
| Some(table_config) => Ok(table_config), | ||
| None => Err(ExecuteError::wrap("invalid config data")), | ||
| } | ||
| } | ||
| Err(error) => match error.kind() { | ||
| ErrorKind::NotFound => Err(ExecuteError::wrap("table not found")), | ||
| _ => Err(ExecuteError::wrap(format!("{:?}", error))), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // 데이터 저장 경로를 반환합니다.. | ||
| pub fn get_data_directory(&self) -> PathBuf { | ||
| PathBuf::from(self.config.data_directory.clone()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
get_table_config에서 database_name.unwrap() 사용은 패닉 위험이 있습니다
let TableName {
database_name,
table_name,
} = table_name;
let database_name = database_name.unwrap();위 코드에서 database_name이 None인 경우(예: USE db 후 DELETE FROM table처럼 현재 DB 컨텍스트를 이용하려는 경우) 런타임 패닉이 발생할 수 있습니다.
이 레이어에 도달하는 TableName이 항상 Some이라는 불변식을 100% 보장하지 않는다면, 다음과 같이 명시적으로 에러를 반환하는 편이 안전합니다:
- let TableName {
- database_name,
- table_name,
- } = table_name;
-
- let database_name = database_name.unwrap();
+ let TableName {
+ database_name,
+ table_name,
+ } = table_name;
+
+ let database_name = match database_name {
+ Some(name) => name,
+ None => {
+ return Err(ExecuteError::wrap("database is not selected"));
+ }
+ };이렇게 하면 DB 미선택 상태에서의 접근을 명시적인 에러로 처리할 수 있고, 패닉 없이 상위 계층에서 적절히 대응할 수 있습니다.
I'll verify the review comment by examining the codebase for the TableName struct definition and how get_table_config is called.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find TableName struct definition
rg -n "struct\s+TableName" --type rust
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for get_table_config call sites
rg -n "get_table_config" --type rust -B2 -A2Length of output: 3561
🏁 Script executed:
#!/bin/bash
# Search for where database_name is set in TableName construction
rg -n "TableName\s*\{" --type rust -B3 -A3Length of output: 34737
---
🏁 Script executed:
```shell
#!/bin/bash
# Check select.rs handler context around get_table_config call
cat src/engine/actions/dml/select.rs | head -60 | tail -30
Length of output: 1089
🏁 Script executed:
#!/bin/bash
# Check delete.rs handler context around get_table_config call
cat src/engine/actions/dml/delete.rs | head -45 | tail -25Length of output: 786
🏁 Script executed:
#!/bin/bash
# Check update.rs handler context around get_table_config call
cat src/engine/actions/dml/update.rs | head -50 | tail -30Length of output: 1009
🏁 Script executed:
#!/bin/bash
# Search for any database context or validation before these calls
rg -n "database.*context\|current.*database\|USE\s+database" --type rust -i src/engine/actions/dml/ -B2 -A2Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search optimizer for any database_name validation/normalization
rg -n "database_name" src/engine/optimizer/ --type rust -B2 -A2
Length of output: 0
get_table_config에서 database_name.unwrap() 호출 시 런타임 패닉 위험
src/engine/ast/types/table.rs의 TableName 구조체에서 database_name은 Option<String>이며, 파서 테스트 코드(src/engine/parser/test/select.rs:234, 306, 380 등)가 database_name: None인 경우를 명시적으로 구성합니다. DML 핸들러들(src/engine/actions/dml/select.rs:45, delete.rs:37, update.rs:43)이 이러한 테이블명을 검증 없이 get_table_config에 전달하므로, src/engine/mod.rs:111의 database_name.unwrap()은 패닉을 발생시킬 수 있습니다.
let table_name = from.table_name.clone();
let table_config = self.get_table_config(table_name.clone()).await?;위와 같은 호출에서 table_name.database_name이 None이면 패닉이 발생합니다. 명시적인 에러 처리로 변경하세요:
- let database_name = database_name.unwrap();
+ let database_name = match database_name {
+ Some(name) => name,
+ None => {
+ return Err(ExecuteError::wrap("database is not selected"));
+ }
+ };🤖 Prompt for AI Agents
In src/engine/mod.rs around lines 101 to 138, replace the direct
database_name.unwrap() with explicit Option handling to avoid a runtime panic:
match or if-let on table_name.database_name, and if it's None return an
appropriate Err(RRDBError) (e.g. Err(ExecuteError::wrap("missing database
name"))), otherwise use the unwrapped value; keep the rest of the function
unchanged and ensure the error type matches the function's Result so callers
receive a clear, handled error instead of panicking.
| use crate::engine::ast::ddl::alter_table::{ | ||
| AlterColumnDropDefault, AlterColumnDropNotNull, AlterColumnSetDefault, AlterColumnSetNotNull, | ||
| AlterColumnSetType, AlterTableAddColumn, AlterTableAlterColumn, AlterTableDropColumn, | ||
| AlterTableQuery, AlterTableRenameColumn, AlterTableRenameTo, | ||
| }; | ||
| use crate::ast::ddl::create_table::CreateTableQuery; | ||
| use crate::ast::ddl::drop_database::SQLStatement; | ||
| use crate::ast::ddl::drop_table::DropTableQuery; | ||
| use crate::engine::ast::ddl::create_table::CreateTableQuery; | ||
| use crate::engine::ast::ddl::drop_database::SQLStatement; | ||
| use crate::engine::ast::ddl::drop_table::DropTableQuery; | ||
| use crate::errors::predule::ParsingError; | ||
| use crate::errors::RRDBError; | ||
| use crate::lexer::predule::Token; | ||
| use crate::parser::context::ParserContext; | ||
| use crate::parser::predule::Parser; | ||
| use crate::engine::lexer::predule::Token; | ||
| use crate::engine::parser::context::ParserContext; | ||
| use crate::engine::parser::predule::Parser; |
There was a problem hiding this comment.
SQLStatement import 경로가 잘못되었을 가능성이 큽니다.
현재 이 파일에서는 다음과 같이 SQLStatement 를 가져오고 있습니다.
use crate::engine::ast::ddl::drop_database::SQLStatement;하지만 다른 파서/테스트 코드에서는 일관되게 crate::engine::ast::SQLStatement 를 사용하고 있고, drop_database 모듈이 SQLStatement 를 정의하거나 재export 하는 구조는 이름상으로도 자연스럽지 않습니다. 이 상태라면:
drop_database모듈에SQLStatement가 없으면 컴파일 에러가 발생하거나,- 같은 이름의 전혀 다른 타입을 잘못 import 하게 될 위험이 있습니다.
이 파일에서도 아래와 같이 루트 AST에서 직접 가져오는 쪽이 의도에 더 맞고, 다른 코드들과도 일관적일 것 같습니다.
-use crate::engine::ast::ddl::drop_database::SQLStatement;
+use crate::engine::ast::SQLStatement;또한 ParserContext 를 이 파일만 parser::context::ParserContext 에서 직접 가져오고, 다른 구현은 parser::predule 경유로 쓰고 있어서, 나중에 정리하실 때는 prelude 경로로 통일하는 것도 고려해 보시면 좋겠습니다.
🤖 Prompt for AI Agents
In src/engine/parser/implements/ddl/table.rs around lines 1 to 13, the file
incorrectly imports SQLStatement from
crate::engine::ast::ddl::drop_database::SQLStatement which likely points to the
wrong module; change the import to use the canonical
crate::engine::ast::SQLStatement so the parser uses the root AST type
consistently with other modules, and while here also change the ParserContext
import to come from crate::engine::parser::prelude (or the same prelude path
used elsewhere) to keep imports consistent.
2 participants
resolves: #176
설명
가독성, 유지보수를 고려해서 계층 구조로 재구성
Summary by CodeRabbit
릴리스 노트
New Features
Refactor