Cbrd 26217 histogram #6653
Cbrd 26217 histogram #6653
Conversation
:빌드 되게 만들고 나머지 TODO 및 ASSERT 처리
- 중간부 구현: SERVER_SIDE로 가기 직전의 빌드 가능하고, 실행 가능한 상태로 중간 구현
- histogram 관련 오류 수정 (db_class fetch 방법 관련)
…/cubrid into CBRD-26217-histogram # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다.
class 전체의 meta 정보가 histogram에 포함되지 않도록 변경
|
@codex 이 코드는 다 완성된 코드가 아니고, 추가적으로 TODO Implement가 꽤 많이 포함되어 있어 이 모든 것은 한국어로 해줘 |
|
To use Codex here, create an environment for this repo. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex 이 코드는 다 완성된 코드가 아니고, 추가적으로 TODO Implement가 꽤 많이 포함되어 있어 이 모든 것은 한국어로 해줘 |
|
To use Codex here, create an environment for this repo. |
|
@codex 이 코드는 다 완성된 코드가 아니고, 추가적으로 TODO Implement가 꽤 많이 포함되어 있어 이 모든 것은 한국어로 해줘 |
|
@codex 이 코드는 다 완성된 코드가 아니고, 추가적으로 TODO Implement가 꽤 많이 포함되어 있어 이 모든 것은 한국어로 해줘 |
|
@codex 리팩토링 관련 리뷰 부탁할게 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| attr_dom = tp_domain_copy ((*attr)->domain, false); | ||
| assert (attr_dom->type->id <= DB_TYPE_LAST); | ||
| if (attr_dom == NULL) |
There was a problem hiding this comment.
In src/query/execute_statement.c lines 11965-11967, the new assert(attr_dom->type->id <= DB_TYPE_LAST); dereferences attr_dom immediately after tp_domain_copy without checking for NULL. If tp_domain_copy fails (e.g., OOM), execution will segfault before the existing error handling at line 11967 can run. The null check needs to precede any dereference to keep the error path safe.
Useful? React with 👍 / 👎.
| SM_CLASS *class_ = NULL; | ||
| error = au_fetch_class (obj, &class_, AU_FETCH_READ, AU_SELECT); | ||
| for (att = class_->attributes; att != NULL; att = (SM_ATTRIBUTE *) att->header.next) | ||
| { |
There was a problem hiding this comment.
In src/query/execute_schema.c lines 3908-3911, au_fetch_class is called but its return value is ignored; the subsequent for loop immediately dereferences class_->attributes. If the fetch fails (invalid class, permissions, or out of memory), class_ is left unset and this path will crash instead of returning the error. The fetch result should be checked and returned before iterating over class_.
Useful? React with 👍 / 👎.
| assert (oid_count < 2); | ||
| return obj; |
There was a problem hiding this comment.
In src/object/object_accessor.c lines 3814-3815, oid_count is asserted to be less than 2 even when control reaches end_find before btree_find_multi_uniques runs (e.g., allocation failures or invalid arguments). In those paths oid_count is never initialized, so the assertion reads indeterminate memory and can crash debug builds. Initialize oid_count or guard the assertion so it only runs after the search populates it.
Useful? React with 👍 / 👎.
|
@codex 리뷰해 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (len32 <= 4) // inline data | ||
| { | ||
| return std::string_view{ p+4, static_cast<std::size_t> (len32-4) }; | ||
| } |
There was a problem hiding this comment.
When reading string histograms, inline values (length ≤ 4) are deserialized with len32-4 as their length, but the writer stored the full length and copied the characters directly. Any bucket whose endpoint is a short string therefore returns an empty/garbled std::string_view, corrupting equality planning for those values. Use the recorded len32 for inline strings instead of subtracting four bytes.
Useful? React with 👍 / 👎.
src/histogram/histogram_cl.cpp
Outdated
| *selectivity = (static_cast<double> (histogram_reader.bucket_rows (bucket_index - 1)) / static_cast<double> | ||
| (histogram_reader.total_rows())) / | ||
| static_cast<double> (histogram_reader.bucket_approx_ndv (bucket_index)); |
There was a problem hiding this comment.
Selectivity calculation always uses bucket_rows(bucket_index - 1), so when find_bucket returns 0 (the first bucket) the index underflows and triggers the bucket_rows bounds assert or yields garbage, producing a crash or meaningless selectivity for the smallest values. Guard the zero case or use the current bucket instead of bucket_index - 1.
Useful? React with 👍 / 👎.
…/cubrid into CBRD-26217-histogram # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다.
- CMP 로직에 해당 사항 추가함. - MCV bucket이라면 뒤로 후퇴할 것
…/cubrid into CBRD-26217-histogram
- semantic_check시 invalid type을 포함하지 않도록 수정 - query_dump 추가 - 일부 업데이트에 대한 기능 추가 (해당되는 컬럼에 대해 이미 있어도 하도록)
|
@codex 한국말로 리뷰해줘 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (error != NO_ERROR) | ||
| { | ||
| assert (er_errid () != NO_ERROR); | ||
| error = er_errid (); | ||
| return error; |
There was a problem hiding this comment.
이 함수는 4044행에서 AU_DISABLE(save)로 권한 검사를 끄지만, 클래스 조회 실패나 update_or_drop_histogram_helper 오류 시 4061~4065에서 바로 반환하여 4068의 AU_ENABLE(save)가 실행되지 않습니다. 이 경로를 한 번이라도 타면 세션 전체의 AU 상태가 영구히 비활성화되어 이후 작업들이 권한 검사를 우회하거나 예기치 않은 오류를 낼 수 있습니다. 실패 시에도 항상 AU 상태를 복구하도록 정리 루프를 추가해야 합니다.
Useful? React with 👍 / 👎.
| if (error != NO_ERROR) | ||
| { | ||
| assert (er_errid () != NO_ERROR); | ||
| error = er_errid (); | ||
| return error; |
There was a problem hiding this comment.
do_drop_histogram도 4085행에서 AU_DISABLE(save)로 권한을 비활성화한 뒤, 4102~4106에서 오류 시 즉시 반환하며 4109의 AU_ENABLE(save)를 건너뜁니다. 이 경우 세션의 AU 상태가 복구되지 않아 이후 모든 요청이 권한 검사를 우회하게 됩니다. 오류 반환 전에 AU 상태를 복원하는 정리 경로가 필요합니다.
Useful? React with 👍 / 👎.
1 participant
http://jira.cubrid.org/browse/CBRD-XXXX
Purpose
N/A
Implementation
N/A
Remarks
N/A