기여 목록 / 8529226
[sql] Remove migration TODOs from ColumnTime() and ColumnTimeDelta()
sql::Statement::ColumnTime()·ColumnTimeDelta() 주석에 남아 있던 손 변환 목록과 TODO 두 개를 지우고, 두 접근자가 하는 일을 설명하는 주석으로 바꿨습니다. sql::Statement 시간 접근자 이행 시리즈(crbug.com/40176243, OSSCA #416)의 일곱 번째이자 마지막 CL입니다. 1파일 +9/−16, 주석만 바뀝니다. 첫 리뷰어 grt@가 승인하면서 BindTime()·BindTimeDelta() 주석처럼 «무엇을 하고 어떤 손 변환 대신 쓰는지»를 적어 달라고 요청해 PS2에서 주석을 새로 썼습니다. 두 번째 리뷰어 evanstade@가 승인하고 CQ를 돌려 2026-10-08(UTC)에 머지됐습니다(1e57ac1cef932).
문제 설명
- issue url: https://crbug.com/40176243, https://crbug.com/40251269
sql/statement.h의 두 주석은 이 접근자로 바꿔야 할 손 변환(base::Time::FromInternalValue(ColumnInt64(col)),base::Time::FromDeltaSinceWindowsEpoch(base::Microseconds(ColumnInt64(col))),base::TimeDelta::FromInternalValue(ColumnInt64(column_index)))을 나열하고, 호출하는 곳을 모두 옮긴 뒤 이 목록을 지우라는 TODO를 달고 있었습니다.- grt@의 7455767이 한 줄짜리
BindInt64()쓰기를BindTime()·BindTimeDelta()로 옮겼고, 이 시리즈의 CL 1~6(8382856, 8410045, 8474691, 8479611, 8505857, 8521677)이 남은 읽기와 쓰기를 옮겼습니다. 여섯 개가 모두 머지되어 TODO가 요구한 일이 끝났습니다.
해결 내용
두 주석에서 손 변환 목록과 TODO를 지웠습니다. 첫 리뷰에서 grt@가 같은 파일의 BindTime()·BindTimeDelta() 주석(7455767에서 고친 형태)처럼 접근자가 하는 일과 대체하는 손 변환을 적자고 제안해, PS2에서 그 형태로 바꿨습니다. ColumnTime() 주석에 «serialization»으로 잘못 적혀 있던 것도 이때 «Deserializes»로 바로잡혔습니다. ColumnTimeDelta()도 같은 형태로 바꿨습니다.
- // Conforms with base::Time serialization recommendations.
- //
- // This is equivalent to the following snippets, which should be replaced.
- // * base::Time::FromInternalValue(ColumnInt64(col))
- // * base::Time::FromDeltaSinceWindowsEpoch(
- // base::Microseconds(ColumnInt64(col)))
- //
- // TODO(crbug.com/40176243): Migrate all time serialization to this method,
- // and then remove the migration details above.
+ // Deserializes a `base::Time` in full fidelity (the number of microseconds
+ // since the Windows epoch in UTC). This form of retrieval is preferred over
+ // `base::Time::FromInternalValue(ColumnInt64(column_index))` or
+ // `base::Time::FromDeltaSinceWindowsEpoch(
+ // base::Microseconds(ColumnInt64(column_index)))`.
base::Time ColumnTime(int column_index);아직 손으로 변환하는 곳은 CL 설명에 이유와 함께 적었습니다. 모두 이 접근자가 다루지 않는 경우입니다.
- 자동완성
PaymentsAutofillTable의 오퍼 만료 시각은 밀리초 단위로 저장합니다. HistoryDatabase와 IndexedDB SQLite 백엔드는 시각을sql::MetaTable에,BtmDatabase는 자체 config 테이블에 둡니다. 둘 다 범용 int64 키-값 저장소입니다.- record_replay
TaskDatabase는 시각을 protobuf int64 필드로 다루므로 정리 기준 시각도 int64로 바인딩합니다. VisitDatabase는 끝이 열린 질의의 종료 시각을 int64 최댓값으로 바인딩합니다. 질의 경계에 쓰는 특수값입니다.
발굴 과정
CL 6이 머지된 뒤 최신 main(148fce6cf91ed)에서 전수 재검색을 했습니다. 줄바꿈으로 나뉜 문장을 합치고 지역 변수를 거치는 읽기·쓰기까지 따라가는 스캔과, sql을 쓰는 비테스트 파일 113개에서 FromInternalValue(·FromDeltaSinceWindowsEpoch(·ToInternalValue()·ToDeltaSinceWindowsEpoch()를 찾는 스캔을 함께 돌렸습니다. 테스트를 빼면 시간 컬럼을 손으로 읽는 곳은 0곳이었습니다. 변환 함수가 남은 9개 파일은 접근자 구현(sql/statement.cc) 자신과 위 예외, 그리고 sql 문장 밖의 쓰임(저장 키 문자열, ColumnTime()으로 읽은 뒤의 변환)이었습니다. 업로드 직전 main(8eacc64f42746, 651커밋 뒤)에서 다시 돌려도 결과가 같았습니다.
리뷰어는 sql/OWNERS 중 1부(7455767)를 쓰고 두 버그를 닫은 grt@에게 먼저 받았습니다. 주석을 고친 PS2를 올리면서 같은 OWNERS의 evanstade@를 두 번째 리뷰어로 더했고, grt@가 PS2를 다시 승인한 뒤 evanstade@도 승인했습니다.
테스트 방법
주석만 바꾸는 변경이라 빌드와 테스트는 생략했습니다. 대신 위 전수 재검색으로 남은 손 변환이 없다는 것을 확인했습니다.
git cl presubmit --upload오류 0 (PS1·PS2)- clang-format 결과 변경 없음
배운 점
- TODO를 지우는 CL은 «정말 끝났는지»를 보이는 것이 핵심입니다. 단순 grep으로는 여러 줄에 걸친 문장이나 지역 변수를 거치는 변환을 놓칠 수 있어, 문장 단위로 합쳐 보는 스캔을 따로 만들었습니다.
- 남은 예외를 이유와 함께 CL 설명에 적어 두면 리뷰어가 직접 다시 검색하지 않고도 판단할 수 있습니다.
- 빌드가 필요 없는 변경은 필요한 디렉터리만 체크아웃한 sparse worktree(61MB)에서 올릴 수 있어 큰 빌드 트리를 건드리지 않습니다. presubmit이 불러오는
build/util·tools/android/infobar_deprecation같은 디렉터리는 함께 체크아웃해야 합니다. - TODO를 지울 때는 남는 주석이 무엇을 말해야 하는지도 함께 봅니다. 목록만 지우면 «Conforms with … recommendations» 한 줄만 남는데, grt@는 같은 파일의 짝 함수처럼 «무엇을 하고 어떤 손 변환 대신 쓰는지»를 적자고 했습니다.
- 리뷰 중 받은 제안을 반영했다면 CL 설명에 그 출처를 적어 둡니다. 두 번째 리뷰어가 새 주석에 «구현 세부를 헤더에 쓸 필요는 없다»는 nit을 달았다가, 첫 리뷰어의 제안이었다는 것을 보고 거둬들였습니다.