Skip to content

CROSSLINK-210 Add date time field support#39

Merged
JanisSaldabols merged 1 commit intomainfrom
CROSSLINK-210
Mar 5, 2026
Merged

CROSSLINK-210 Add date time field support#39
JanisSaldabols merged 1 commit intomainfrom
CROSSLINK-210

Conversation

@JanisSaldabols
Copy link
Collaborator

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds PostgreSQL date/datetime term parsing and query generation to the pgcql package, with accompanying unit and integration tests to validate SQL generation and execution.

Changes:

  • Introduces a new FieldDateTime field type (NewFieldDate, WithOnlyDate) to generate SQL predicates and bind time.Time arguments.
  • Extends parsing tests to cover date-only, datetime, and RFC3339-with-timezone inputs.
  • Extends pgx integration tests to exercise date and timestamp columns and related CQL queries.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pgcql/pg_field_datetime.go Adds the new date/datetime Field implementation and parsing logic.
pgcql/pgcql_test.go Adds unit tests asserting SQL and bound args for date/datetime parsing.
pgcql/pgx_test.go Adds integration coverage for date/timestamp columns and comparisons.
Comments suppressed due to low confidence (2)

pgcql/pg_field_datetime.go:42

  • The local variable name number holds a time.Time value from parseTerm, which is misleading and makes the code harder to follow. Rename it to something time-related (e.g., ts, dt, parsedTime) to match its actual type and intent.
	number, err := f.parseTerm(sc.Term)
	if err != nil {

pgcql/pg_field_datetime.go:20

  • NewFieldDate() constructs a type that (by default) accepts date and datetime inputs; the name suggests a date-only field. Consider renaming this constructor to something like NewFieldDateTime() (and optionally adding a dedicated NewFieldDate() that sets WithOnlyDate()), to avoid confusing API consumers and make the default behavior explicit.
func NewFieldDate() *FieldDateTime {
	return &FieldDateTime{}
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

@adamdickmeiss adamdickmeiss left a comment

Choose a reason for hiding this comment

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

Looks good.

Perhaps allowing for only year to be given.

Also perhaps make support for date range with a 'within' operator.

@JanisSaldabols JanisSaldabols merged commit da2d00e into main Mar 5, 2026
5 checks passed
@JanisSaldabols JanisSaldabols deleted the CROSSLINK-210 branch March 5, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants