Skip to content

[chore](skill) Add nullable handling AI review rules#64442

Merged
zclllyybb merged 2 commits into
apache:masterfrom
zclllyybb:nullable
Jun 12, 2026
Merged

[chore](skill) Add nullable handling AI review rules#64442
zclllyybb merged 2 commits into
apache:masterfrom
zclllyybb:nullable

Conversation

@zclllyybb

Copy link
Copy Markdown
Contributor

followup of #64407

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@zclllyybb zclllyybb marked this pull request as ready for review June 12, 2026 04:17
@zclllyybb

Copy link
Copy Markdown
Contributor Author

/review

@zclllyybb

Copy link
Copy Markdown
Contributor Author

run buildall

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review result: no blocking issues found.

Critical checkpoint conclusions:

  • Goal and proof: This PR adds high-priority BE nullable/ColumnConst review rules to .claude/skills/code-review/SKILL.md. The added guidance matches the actual core column behavior I checked: is_column_nullable() is a concrete ColumnNullable type check, ColumnConst::is_nullable() can report nullable when its nested data is nullable, and remove_nullable(ColumnConst(ColumnNullable(T))) preserves the const wrapper as ColumnConst(T). No runtime test is required for this documentation-only skill update; git diff --check passed.
  • Scope: The modification is small and focused: one review skill file, 9 added lines.
  • Concurrency: Not applicable; no runtime code, shared state, locks, or thread entry points changed.
  • Lifecycle/static initialization: Not applicable; no C++ objects or initialization paths changed.
  • Configuration: Not applicable; no config item changed.
  • Compatibility/storage/protocol: Not applicable; no function symbols, storage formats, Thrift fields, FE-BE protocol, or persisted metadata changed.
  • Parallel paths: The existing function-system null-handling checklist remains valid; the new section generalizes the same nullable/const-column trap for BE column review and does not conflict with it.
  • Conditions/error handling/memory/data correctness/observability/performance: Not runtime-affecting. The guidance accurately targets review-time risks around nullable column shape, const materialization, and invariant handling.
  • Tests and result files: No regression, unit, or .out files are changed, which is appropriate for this skill/documentation-only PR.
  • User focus: No additional user-provided review focus was present.

@zclllyybb zclllyybb merged commit a20a0df into apache:master Jun 12, 2026
34 checks passed
@zclllyybb zclllyybb deleted the nullable branch June 12, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants