Skip to content

chore(BigQuery): Clarifying supported date/time format in examples#8545

Merged
cy-yun merged 1 commit intogoogleapis:mainfrom
cy-yun:load_job_configuration_fix
Aug 29, 2025
Merged

chore(BigQuery): Clarifying supported date/time format in examples#8545
cy-yun merged 1 commit intogoogleapis:mainfrom
cy-yun:load_job_configuration_fix

Conversation

@cy-yun
Copy link
Copy Markdown
Contributor

@cy-yun cy-yun commented Aug 28, 2025

  • learned from writing Ruby acceptance tests and conversation with @shollyman that C-style date/time format is not supported. Updated docs and examples to reflect this finding.

@cy-yun cy-yun requested review from a team August 28, 2025 20:42
@product-auto-label product-auto-label Bot added the api: bigquery Issues related to the BigQuery API. label Aug 28, 2025
Comment thread BigQuery/src/LoadJobConfiguration.php Outdated
* @return LoadJobConfiguration
*/
public function columnNameCharacterMap(string $columnNameCharacterMap): self
public function columnNameCharacterMap(string $columnNameCharacterMap)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question:
I see that you decided to remove the self. Was this causing issues for some reason? We are currently trying to enforce return types on new code (And want to update old code as well) but I want to know the reason why this return type is getting removed :).

Maybe there's something I am missing and need documenting.

Copy link
Copy Markdown
Contributor Author

@cy-yun cy-yun Aug 28, 2025

Choose a reason for hiding this comment

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

They weren't causing any issues. I added these setters in a recent PR (#8468). I was reviewing this file while correcting the examples on the date/time format and noticed that previously existing setters do not have a return type (I thought PHP implies static return type in this case, but I was misinformed. It wouldn't assume any particular return type). I asked Gemini what return type would be best practice (self vs static) and it answered:

In PHP, self and static are used for type hinting, but they behave differently
  with inheritance.

   - `self`: Refers to the exact class in which the type hint is written.
   - `static`: Refers to the class that is called at runtime. This is known as
     "late static binding."

  The difference matters when a class is extended. If a method in a parent class
  returns self, type-hinting systems will infer the return type as the parent class,
   even if the method is called on a child class instance. If it returns static, the
   type will be correctly inferred as the child class.

  Using static is generally preferred for fluent interfaces in classes that might be
   extended, as it allows for better static analysis and autocompletion on child
  classes.

I'll fix this PR such that we use static return type for all of the setters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nvm, changing return types is a breaking change. I'll just change the examples

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

Yeah we are on our way of typing everything. This codebase is a bit old so not everything is typing, so we really appreciate the return types.

If you don't define a return type, it basically is returning a mixed type to my understanding. Which means that you lose typing and you lose autocomplete on your IDE.

@cy-yun cy-yun force-pushed the load_job_configuration_fix branch from 07ac0c1 to 57db482 Compare August 28, 2025 21:06
@cy-yun cy-yun force-pushed the load_job_configuration_fix branch from 57db482 to 84c27ba Compare August 28, 2025 22:21
@cy-yun cy-yun changed the title chore(BigQuery): Making setters in LoadJobConfiguration static and clarifying supported date/time format in examples chore(BigQuery): Clarifying supported date/time format in examples Aug 28, 2025
@cy-yun cy-yun merged commit 0517f53 into googleapis:main Aug 29, 2025
35 checks passed
purva9413 pushed a commit to purva9413/google-cloud-php that referenced this pull request Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants