text:
nrf5340: avoid have 2 interrupts for each IPC message

The Nimble IPC messages are written in 2 parts: the cmd byte and the rest.
It is unneccesary to already interrupt the other core for the first byte,
since it will be interrupted by the 2nd write already.
(a 2nd patch to Nimble will use this new function)

Style check summary
Our coding style is here!
hw/drivers/ipc_nrf5340/src/ipc_nrf5340.c

@@ -405,12 +405,14 @@
         /* assert since that will always fail for non-blocking write
          * indicating use error
          */
-        if (space == 0) return SYS_ENOMEM;
+        if (space == 0) {
+            return SYS_ENOMEM;
+        }
 #endif
     } while (space == 0);

     ipc_nrf5340_shm_write(shm, &data, 1);
-    // dont notify other side! (will be done by later calls to ipc_nrf5340_send)
+    /* dont notify other side! (will be done by later calls to ipc_nrf5340_send) */

     return 0;
 }

@bvdberg I think that ipc_nrf5340_send() is already used by some users (not only by nimble and what is in mynewt core) that's why I propose to have another function and send ipc_nrf5340_send() would just used it.
Changing prototype is not a way to go I think.
I'm sorry if it was not clear in my previous comment.

sounds fair, i have added ipc_nrf5340_write() with the bool last argument. The ipc_nrf5340_send() now uses write(.., true)

doh! I did build though. Ok fixed :)

Style check summary
No suggestions at this time!

This will break blocking send if data is bigger than internal shm size... I think notification should stil be done from withing loop (note that for non-blocking config this loop iterates only once anyway)

Maybe changing from:
    NRF_IPC->TASKS_SEND[channel] = 1;

to:
    if (last || len > space) {
        NRF_IPC->TASKS_SEND[channel] = 1;
    }

Style check summary
No suggestions at this time!

Well spotted. Modified as @kasjer proposed.

Style check summary
No suggestions at this time!

I made all the changes, new it should be perfect ;)

authors: bvdberg kasjer utzig

repo: apache/mynewt-core


text:
Ios

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:

[ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?

[ ] Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

[ ] Has your PR been rebased against the latest commit within the target branch (typically master)?

[ ] Is your initial contribution a single, squashed commit?

For code changes:

[ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
[ ] If applicable, have you updated the LICENSE file?
[ ] If applicable, have you updated the NOTICE file?

For documentation related changes:

[ ] Have you ensured that format looks appropriate for the output in which it is rendered?

Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

reviewing

source: gharchive/pull-request

repo: apache/nifi-minifi-cpp

license_type: permissive

license_source: github-api


text:
NIFI-5542 Added support for node groups to FileAccessPolicyProvider

Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:

[x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?

[x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

[x] Has your PR been rebased against the latest commit within the target branch (typically master)?

[x] Is your initial contribution a single, squashed commit?

For code changes:

[x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
[x] Have you written or updated unit tests to verify your changes?
[x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
[x] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
[x] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
[x] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

[x] Have you ensured that format looks appropriate for the output in which it is rendered?

Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

@pepov @mcgilman new PR where we authorize the node group rather than its users. Please take a look when you get a chance.

+1 looks good to me now

Thanks for pointing this out! Of course we need identity here, but if I'm right identity is called name in case of groups.

I'm not trying to suggest naming just to point out that the Group has the name property instead of the identity that the User has. The workaround should be as simple as looping through the groups and finding the one with a matching name, am I right?

Yes, that should work fine to lookup group by name. Good point regarding the field name of the Group object. Thanks for looking into this!

@pepov @kevdoran thanks for the feedback. I'll update the docs to reflect the property change, and update the code to reflect that the group name is provided and look up the group identifier accordingly.

Some unit tests are failing according to the travis build that are relevant:
[ERROR] testOnConfiguredWhenInitialAdminAndLegacyUsersProvided(org.apache.nifi.authorization.FileAccessPolicyProviderTest)  Time elapsed: 0.114 s  <<< ERROR!
java.lang.Exception: Unexpected exception, expected<org.apache.nifi.authorization.exception.AuthorizerCreationException> but was<java.lang.NullPointerException>
	at org.apache.nifi.authorization.FileAccessPolicyProviderTest.testOnConfiguredWhenInitialAdminAndLegacyUsersProvided(FileAccessPolicyProviderTest.java:523)

[ERROR] testOnConfiguredWhenBadLegacyUsersFileProvided(org.apache.nifi.authorization.FileAccessPolicyProviderTest)  Time elapsed: 0.036 s  <<< ERROR!
java.lang.Exception: Unexpected exception, expected<org.apache.nifi.authorization.exception.AuthorizerCreationException> but was<java.lang.NullPointerException>
	at org.apache.nifi.authorization.FileAccessPolicyProviderTest.testOnConfiguredWhenBadLegacyUsersFileProvided(FileAccessPolicyProviderTest.java:509)

https://api.travis-ci.org/v3/job/422545351/log.txt

The group id lookup logic was running even if group name was not specified. Fixed that. All tests are passing locally.

Nitpicking but now we do 2 lookups. First we look up the group to get the identifier then we lookup the same group using that identifier. I beleive it would be enough to store the group name property and look it up in the populateNodes method only, but this is nitpicking, since there is no performance or memory penalty in it, so overall this looks good to me.

@pepov is the second lookup you're referring to this line?

            Group nodeGroup = userGroupProvider.getGroup(nodeGroupIdentifier);

yes

@pepov I took that lookup out. It appeared to be completely unnecessary.

Even better, thanks!

@kevdoran Are you okay with the current state? Also @mcgilman can you take a look at this?

@kevdoran @achristianson created a follow up PR to avoid an issue with the default empty "Node Group" in authorizers.xml: https://github.com/apache/nifi/pull/3043

authors: achristianson kevdoran pepov

repo: apache/nifi


text:
NIFI-8119: properly free database resources when done with the processing of BLOBs

Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
Description of PR
Fixes bug NIFI-8119.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:

[x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?

[x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

[x] Has your PR been rebased against the latest commit within the target branch (typically main)?

[x] Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not squash or use --force when pushing to allow for clean monitoring of changes.

For code changes:

[ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
[ ] Have you written or updated unit tests to verify your changes?
[ ] Have you verified that the full build is successful on JDK 8?
[x] Have you verified that the full build is successful on JDK 11?
[ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
[ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
[ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
[ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

[ ] Have you ensured that format looks appropriate for the output in which it is rendered?

Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.

This is pretty straightforward so I'm a +1, but since Derby supports BLOBs, I was hoping to see a unit test even if only for coverage purposes. If it's more trouble than it's worth, let me know and I'll merge as-is (after I test on various DBs). Thanks!

+1 LGTM, I looked into unit tests but Derby makes BLOB stuff very painful. Since all InputStreams should be closed when processing is finished, I'm good with this as-is. Thanks for the improvement! Merging to main


text:
NIFI-10141: Fix WriteJsonResult logging to show writer's schema

Summary
NIFI-10141 The current error logging when a field cannot be written out uses the record's (i.e the record reader's) schema, which is not an accurate depiction of why the field cannot be written. Instead the record writer's schema should be used in the logging statement to show the schema used to write the record.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking

[x] Apache NiFi Jira issue created

Pull Request Tracking

[x] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
[x] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

[x] Pull Request based on current revision of the main branch
[x] Pull Request refers to a feature branch with one commit containing changes

Verification
Please indicate the verification steps performed prior to pull request creation.
Build

[ ] Build completed using mvn clean install -P contrib-check

[x] JDK 8
[ ] JDK 11
[ ] JDK 17

Licensing

[ ] New dependencies are compatible with the Apache License 2.0 according to the License Policy
[ ] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

[ ] Documentation formatting appears as expected in rendered files

@mattyb149 I think it's important to include both the record's schema and the writer's schema. This way, if there's a mismatch it becomes clear

Thanks @mattyb149 +1 merged to main


text:
Fix the issue about the generation of docker image

Summary

Revert "CI: Free space on worker building container", see:
https://github.com/apache/nuttx/pull/11343
ci/docker: Revert the change about gn to unblock ci, reported here:
https://github.com/apache/nuttx/actions/runs/7155266012/job/19483732190?pr=11360)

Impact
docker image
Testing
ci

let me merge the pr to unblock ci.

repo: apache/nuttx


text:
ORC-1362: [Java] Enable ENABLE_INDEXES

What changes were proposed in this pull request?
Enable the functionality behind orc.create.index.
Why are the changes needed?
In https://github.com/apache/orc/pull/1373, we disabled the configuration "orc.create.index" because the orc reader cannot work well when orc.create.index=false.
During the write of a file with orc.create.index=false and orc.row.index.stride > 0 no indexes are created in the ORC file. The orc.row.index.stride is persisted into the file metadata. During a read that requires the use of indexes e.g. in the presence of search arguments we get a read failure as the reader was expecting indexes based on the value of orc.row.index.stride but none exist.
Two changes are proposed here:

Enable consistent writes i.e. assume orc.row.index.stride=0 if orc.create.index=false or if orc.row.index.stride >0 then assume orc.create.index=true
Enable the reader to handle the absence of row index information. During the process of selecting row groups in a stripe if the index information is seen as missing then we assume that all row groups should be read as the index information is missing.

How was this patch tested?
UT

CC @dongjoon-hyun @guiyanakuang @pavibhai

Could you use a different title, @deshanxiao ? This PR only considers Java side, doesn't it?

Done

Hi @pavibhai Sorry for my late update. I have modified all comments you mention. Could you please take a look?

@deshanxiao thanks for the changes. I will get back to you soon.

Thanks for the changes @deshanxiao.
While merging I felt that we should update the description of the PR. I have updated the description of the PR please check to see if this is acceptable.
cc @guiyanakuang @dongjoon-hyun

Thank you so much, @deshanxiao , @guiyanakuang , @pavibhai !
Also, cc @wgtmac , @stiga-huang , @coderex2522 for C++ part compatibility. Please let us know if there is any concern for this.

Additionally, cc @williamhyun

I took a glance and found we may have a similar issue on the C++ side: https://github.com/apache/orc/blob/main/c%2B%2B/src/Writer.cc#L422

Thank you for taking a look at it. Do you think we can have consistent behavior in both C++ and Java languages in Apache ORC 1.9?

Thank you for taking a look at it. Do you think we can have consistent behavior in both C++ and Java languages in Apache ORC 1.9?

Yes, I will fix it later this week.

After a deep look, I can confirm that the C++ side does not have the issue.
The C++ writer only enables writing index with a positive rowIndexStride: https://github.com/apache/orc/blob/main/c%2B%2B/src/Writer.cc#L127
The C++ reader only creates SargsApplier with a positive rowIndexStride in the footer: https://github.com/apache/orc/blob/main/c%2B%2B/src/Reader.cc#L304
@dongjoon-hyun

@wgtmac Thank you for the confirmation! Then, we are good.

Thank you very much! @pavibhai @dongjoon-hyun @guiyanakuang @wgtmac

authors: deshanxiao dongjoon-hyun pavibhai wgtmac

repo: apache/orc

license_source: bigquery


text:
HDDS-4207. Include version information in docs

What changes were proposed in this pull request?
Display version from POM in documentation footer.
If building in a git working copy, also display last commit date and hash, with link to GitHub.
https://issues.apache.org/jira/browse/HDDS-4207
How was this patch tested?
Built docs locally (mvn clean package in hadoop-hdds/docs), both in git repo and in hadoop-ozone-1.0.0-src.  See text on light gray background at the bottom in these screenshots:

Thanks @vivekratnavel for taking a look.

How do I see the version after "mvn clean package"?

open hadoop-hdds/docs/target/classes/docs/index.html if only built hadoop-hdds-docs
open hadoop-ozone/dist/target/ozone-1.1.0-SNAPSHOT/docs/index.html after full build

Thanks @adoroszlai! I also noticed one more thing. The footer does not stick at the bottom of the screen.
For pages with content that do not occupy the full screen, we see the footer with version immediately in the middle of the screen as shown in the screen-shot attached. We should either make this footer stick to the bottom of the screen always or we should this version information in a different space. One alternate space I can think of is the bottom of the left menu bar.

For pages with content that do not occupy the full screen, we see the footer with version immediately in the middle of the screen as shown in the screen-shot attached.

Thanks @vivekratnavel.  I tried, but don't know enough CSS to achieve this.  Can you give me some pointer or hint?

I referred to this post for sticky footer - https://css-tricks.com/couple-takes-sticky-footer/ and pushed a commit.
Please check and let me know how it looks.

I referred to this post for sticky footer - https://css-tricks.com/couple-takes-sticky-footer/ and pushed a commit.
Please check and let me know how it looks.

Thank you very much @vivekratnavel.  It looks great on both long and short pages.

Thanks @vivekratnavel for the review and help with the sticky footer.

authors: adoroszlai vivekratnavel

repo: apache/ozone


text:
HDDS-10450. Add GitHub actions labeler for the reconciliation feature branch.

What changes were proposed in this pull request?
Automatically label PRs targeting the container reconciliation feature branch.
What is the link to the Apache JIRA
HDDS-10450
How was this patch tested?
The workflow will not run until merged. There is a similar workflow running successfully for the ozone-site repo . See the config and workflow there for reference.

This is great idea. I think managing the labels in the master branch and resetting the feature branch should be ok.

Keeping a labeler workflow on master sounds good to me. To clarify, should we add an empty labeler.yml config file to master with the label-pr.yml workflow as is, and then add the reconciliation label on the feature branch? Or should we target this PR as is for the master branch, with the idea that feature branches add their label configs to master and bring them in when they are created or merged/rebased from master?

Or should we target this PR as is for the master branch, with the idea that feature branches add their label configs to master and bring them in when they are created or merged/rebased from master?

I prefer this second option.

Base branch has been updated to master and the file comments updated to reflect the new usage. PTAL!

authors: adoroszlai errose28 kerneltime


text:
HDDS-11331. Fix Datanode unable to report for a long time

What changes were proposed in this pull request?
In some cases, Datanodes were unable to report to the SCM for a long time due to StateContext#pipelineActions being stuck. This PR is intended to try to fix this.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11331
How was this patch tested?
Need to ensure CI execution is successful.

ci : https://github.com/jianghuazhu/ozone/actions/runs/10433754194
@szetszwo , can you help review this PR?
Thanks.

@jianghuazhu @szetszwo Thanks for the contribution! I don't have any issues with this pr, but I don't understand why this change resolves the blocking issue? Why did the original code cause the issue to be blocked?

@slfan1989 @szetszwo @jianghuazhu Here is some information about RATIS, and details of the problem. Perhaps the cause of this problem is ratis. https://issues.apache.org/jira/browse/RATIS-2143

@slfan1989 , @weimingdiit , thanks for filing RATIS-2143.  Let's continue the discussion there.

@slfan1989 , thanks also for reviewing this!

authors: jianghuazhu slfan1989 szetszwo weimingdiit


text:
[doc] Fix CompactionAction doc misspelling

Purpose

Linked issue: close #xxx

Tests

API and Format

Documentation

+1

You need to modify test too.

You need to modify test too.

Done !

+1

repo: apache/paimon


text:
[parquet-cli] Argument error: Illegal character in opaque part at index 2 on Windows

Some commands of parquet-cli don't work on Windows.

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main schema C:\Users\masayuki\Downloads\test.parquet                                                               Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main csv-schema C:\Users\masayuki\Downloads\test.csv --record-name Test
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.csv

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main convert-csv C:\Users\masayuki\Downlo ads\test.csv -o C:\Users\masayuki\Downloads\test-csv.parquet
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.csv

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main convert C:\Users\masayuki\Downloads\ userdata1.avro -o C:\Users\masayuki\Downloads\userdata1.parquet
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\userdata1.avro

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main to-avro C:\Users\masayuki\Downloads\ test.parquet -o C:\Users\masayuki\Downloads\test.avro
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main cat C:\Users\masayuki\Downloads\test.parquet
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main head -n 10 C:\Users\masayuki\Downloa ds\test.parquet
Argument error: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet

The following commands work on Windows.

meta
pages
check-stats
column-index

Reporter: Masayuki Takahashi / @masayuki038
Assignee: Masayuki Takahashi / @masayuki038
PRs and other links:

GitHub Pull Request #627

Note: This issue was originally created as PARQUET-1534. Please see the migration documentation for further details.

Masayuki Takahashi / @masayuki038:
The stacktrace is following:

$ java -cp target/classes;target/dependency/* org.apache.parquet.cli.Main --verbose cat C:\Users\masayuki\Down loads\test.parquet
Argument error
java.lang.IllegalArgumentException: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet
        at java.net.URI.create(Unknown Source)
        at org.apache.parquet.cli.BaseCommand.qualifiedURI(BaseCommand.java:178)
        at org.apache.parquet.cli.BaseCommand.getAvroSchema(BaseCommand.java:379)
        at org.apache.parquet.cli.commands.CatCommand.run(CatCommand.java:66)
        at org.apache.parquet.cli.Main.run(Main.java:147)
        at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:70)
        at org.apache.parquet.cli.Main.main(Main.java:177)
Caused by: java.net.URISyntaxException: Illegal character in opaque part at index 2: C:\Users\masayuki\Downloads\test.parquet
        at java.net.URI$Parser.fail(Unknown Source)
        at java.net.URI$Parser.checkChars(Unknown Source)
        at java.net.URI$Parser.parse(Unknown Source)
        at java.net.URI.<init>(Unknown Source)
        ... 7 more

java.net.URI.create throws IllegalArgumentException.

source: gharchive/issue

repo: apache/parquet-java


text:
More descriptive failure messages in HttpMessageParser

I'm currently investigating an issue where we're suddenly running into the following error: HTTP chunk size exceeds the configured limit of 1048576 bytes.
This is possibly due to an infrastructure change we made which results in a change of behavior when chunked transfer encoding is used. I did not find a way to determine the actual size of the incoming chunks and it would be really helpful if it was part of that error message.
My proposal here would be to extend all relevant errors in HttpMessageParser with extra parameter values. e.g. for the max chunk size, the full hex size could be read before validating against maxChunkSize and then including the size parameter in the error message:
https://github.com/apache/incubator-pekko-http/blob/4f80b1ef7cad2833382a659e7dd0b1a54078d126/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala#L309-L320
Given that on line 318 the actual encountered illegal char is part of the error message, I think having the size in the other error is warranted. Apart from max chunk size there are a few other messages that could be improved such as max chunk extensions and amount of chunk trailer headers.
Any thoughts on this proposal?

probably should have been closed after https://github.com/apache/pekko-http/pull/528 was merged

authors: SkyTrix pjfanning

repo: apache/pekko-http


text:
Avoid re-stringifying strings in JSON_FORMAT function

The JSON_FORMAT function currently just calls JsonUtils::objectToString which in turn calls Jackson Databind's ObjectMapper::writeValueAsStringMethod.
If a JSON object that has already been formatted is passed to this function, the string is "re-stringified" and the result is an escaped string.
For instance, taking the JSON object {"data":{"alias":"student1","age":24,"name":{"full.name":"Peter1","nick.name":"Pete1"}}}. Calling the JSON_FORMAT function on this will result in the string value {"data":{"alias":"student1","age":24,"name":{"full.name":"Peter1","nick.name":"Pete1"}}}.
Calling the JSON_FORMAT function again on this string value will result in the escaped string - "{\"data\":{\"alias\":\"student1\",\"age\":24,\"name\":{\"full.name\":\"Peter1\",\"nick.name\":\"Pete1\"}}}". Passing it through the function again will result in a doubly escaped string - "\"{\\\"data\\\":{\\\"alias\\\":\\\"student1\\\",\\\"age\\\":24,\\\"name\\\":{\\\"full.name\\\":\\\"Peter1\\\",\\\"nick.name\\\":\\\"Pete1\\\"}}}\"".
This can lead to unexpected results in certain scenarios. For instance, let's say that we have a table with a data JSON column with the row - {"data":{"alias":"student1","age":24,"name":{"full.name":"Peter1","nick.name":"Pete1"}}}.
Now suppose we want to add a new derived column alias using the ingestion transform function JSON_PATH_STRING(JSON_FORMAT(data), '$.alias') (via segment reload). Since JSON columns are stored as strings internally, calling JSON_FORMAT on its values will lead to escaped strings. This leads to an NPE here because the JSON_PATH_STRING function returns a null as it will treat the escaped string argument as a literal JSON string value. This minor patch fixes this issue by avoiding re-stringifying strings in JsonUtils::objectToString.
Note that the above behavior is especially confusing because if the ingestion transform function was added during table creation itself, ingestion goes through fine because the argument passed to the JSON_FORMAT function in this case would be the raw HashMap that hasn't been converted to a string yet.
bugfix

Codecov Report
Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 0.00%. Comparing base (59551e4) to head (56f0747).
Report is 413 commits behind head on master.

Files
Patch %
Lines

...he/pinot/common/function/scalar/JsonFunctions.java
0.00%
2 Missing :warning:

Additional details and impacted files
@@              Coverage Diff              @@
##             master   #13097       +/-   ##
=============================================
- Coverage     61.75%    0.00%   -61.76%
=============================================
  Files          2436     2439        +3
  Lines        133233   134146      +913
  Branches      20636    20769      +133
=============================================
- Hits          82274        0    -82274
- Misses        44911   134146    +89235
+ Partials       6048        0     -6048

Flag
Coverage Δ

custom-integration1
?

integration
0.00% <0.00%> (-0.01%)
:arrow_down:

integration1
?

integration2
0.00% <0.00%> (ø)

java-11
?

java-21
0.00% <0.00%> (-61.63%)
:arrow_down:

skip-bytebuffers-false
0.00% <0.00%> (-61.75%)
:arrow_down:

skip-bytebuffers-true
?

temurin
0.00% <0.00%> (-61.76%)
:arrow_down:

unittests
?

unittests1
?

unittests2
?

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

I don't fully follow the example in the description. If the value is already JSON string, user shouldn't call JSON_FORMAT again

I'm not sure if this is the correct behavior.
JSON_FORMAT should be able to convert any object into JSON. If the value itself is abc, then the proper JSON version should be "abc".

That makes sense, I hadn't considered that.

If user wants to generate another level of json over a JSON string, we should still allow that, and the new JSON is also valid

Are there any valid use cases for that?

I don't fully follow the example in the description. If the value is already JSON string, user shouldn't call JSON_FORMAT again

Yeah, I agree in principle but this leads to the confusing behavior that I tried to document in the PR description. Let me try again below.

 Scenario 1

There is a table with two columns in its schema - data (JSON) and alias (STRING). alias is a derived column using the following ingestion transform function: JSON_PATH_STRING(JSON_FORMAT(data), '$.alias').
A sample value is: {"data":{"alias":"student1","age":24,"name":{"full.name":"Peter1","nick.name":"Pete1"}}}.
This works as expected and the above value is ingested fine, with the alias column being set to student1.
In this case, the JSON_FORMAT function is called on the Map value representing data during ingestion, which results in the JSON string and that works fine with the JSON_PATH_STRING function.

 Scenario 2

There is a table with one column in its schema - data (JSON). A sample value that has already been ingested is {"data":{"alias":"student1","age":24,"name":{"full.name":"Peter1","nick.name":"Pete1"}}}.
Now, the user wants to add a new derived column alias with the same logic as the above scenario. Now, the user would expect that the same ingestion transform function should work here as well with segment reload.
If the user updates the table config to add the ingestion transform function: JSON_PATH_STRING(JSON_FORMAT(data), '$.alias') (and also updates the schema to add the new alias column), the segment reload fails with an NPE here.
The reason is that in this case, since the data has already been ingested and stored in a segment, the value that is passed to the JSON_FORMAT function in this case is a String (rather than a Map as in the above scenario) since JSON columns are stored as strings internally. This results in an escaped string value that is treated as a literal JSON string value by the JSON_PATH_STRING function thus resulting in an unexpected null value being returned.

Is this working as expected and documented somewhere? Or should we solve this issue in a different way to what this PR was attempting?

In the given example, the problem is actually from the recursive call of data = JSON_FORMAT(data). I don't think Pinot allows this (maybe I was wrong).
If we configure the ingestion transform to be:

data_json = JSON_FORMAT(data)
alias = JSON_PATH_STRING(data_json, '$,alias')
It should work as expected.

repo: apache/pinot


text:
Extract the common logic for upsert metadata manager

Extract the storage independent common logic for upsert metadata manager into BasePartitionUpsertMetadataManager

LGTM!

authors: Jackie-Jiang KKcorps


text:
Revert "Upgrade cert-manager to 1.5.4 (#204)"

This reverts commit 40a4d50923bd93c960a4ef940930177ca80cb77e (#204).
Supporting cert-manager.io/v1 api version requires changes made in #233 or #238. In addition, the change should be made in a way that it is backwards compatible.

@lhotari Would you be interested in having cert-manager installed as a dependency if enabled, or kept outside this chart? If we require it as a dependency we can use the following to get the version (we will need to loop through in case more dependencies are added in the future).
{{- $certManger := index .Chart.Dependencies 0 -}}
{{ $certManger.Version }}

Then we could use the semver function to get more information about the release and edit the TLS to be backwards compatible.
{{- if semverCompare "^1.5.0"  $certManger.Version -}}

I merged https://github.com/apache/pulsar-helm-chart/pull/233, so we can close this.

authors: lhotari michaeljmarshall toneill818

repo: apache/pulsar-helm-chart


text:
Geo Replica with one producer and two "Shared" Consumers

Describe the bug
In Geo Replica scenario, I have two clusters. My producer send message to cluster A, and I can see message in both clusters as I expected. My consumer consume message from Cluster B with "Shared" Consume type. but if another consumer connect to Cluster A with the same subscription name and "Shared" type, it can also consume message, however I expected it cannot consume the same message because Cluster B consumer received it. Is it normal behavior?
To Reproduce
`package com.pulsar.georeplica;
import org.apache.pulsar.client.api.*;
import org.apache.pulsar.client.impl.auth.AuthenticationBasic;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class GeoReplica {
static PulsarClient client;
static PulsarClient client2;
static Map<String, String> map;
static {
try {
map=new HashMap<>();
map.put("userId","user");
map.put("password","pass");
AuthenticationBasic auth=new AuthenticationBasic();
auth.configure(map);
client = PulsarClient.builder()
.serviceUrl("ClusterB")
.authentication(auth)
.build();
client2 = PulsarClient.builder()
.serviceUrl("ClusterA")
.authentication(auth)
.build();
} catch (PulsarClientException e) {
e.printStackTrace();
}
}
public static void main(String[] msg) throws PulsarClientException {
    GeoReplica mp=new GeoReplica();
    mp.produce("ali");
    mp.singleMessageConsume();

    client.close();
}

public void produce(String msg) throws PulsarClientException {
    List<String> restrictReplicationTo = Arrays.asList(
            "pulsar-cluster-1",
            "pulsar-cluster-2"
    );
    Producer<byte[]> producer = client2.newProducer()
            .topic("persistent://poc-tenant/poc-namespace/navid")
            .producerName("FirstProducer")
            .messageRoutingMode(MessageRoutingMode.SinglePartition)
            .create();

// You can then send messages to the broker and topic you specified:
producer.newMessage()
.value(msg.getBytes())
.send();
producer.close();
}
public void singleMessageConsume() throws PulsarClientException {
    Consumer consumer = client.newConsumer()
            .topic("persistent://poc-tenant/poc-namespace/navid")
            .subscriptionType(SubscriptionType.Shared)
            .subscriptionName("my-subscription1")
            .subscribe();
    // Wait for a message like listener
    while(true) {
    Message msg = consumer.receive();
    String result = "";
    try {
        // Do something with the message
        System.out.println("Message received: " + new String(msg.getData()));
        // Acknowledge the message so that it can be deleted by the message broker
        consumer.acknowledge(msg);
    } catch (Exception e) {
        // Message failed to process, redeliver later
        consumer.negativeAcknowledge(msg);
    }
    result = new String(msg.getData());
    }

}

}
`
Expected behavior
If one consumer from Cluster B receive message, the other one from Cluster A shouldn't receive the same message.

You should enable replicated subscription, but it is not in real time.
https://pulsar.apache.org/docs/en/administration-geo/#replicated-subscriptions

@casuallc I did but still receiving the same message in other cluster
Consumer consumer = client2.newConsumer()
.topic("persistent://poc-tenant/poc-namespace/navid")
.subscriptionType(SubscriptionType.Shared)
.subscriptionName("my-subscription1")
.replicateSubscriptionState(true)
.subscribe();

The subscription stats will snapshot at a specified interval which default is 1 second.
Then this snapshot will send to remote clusters, so you may receive duplicated messages for about 1 second.

@navid1981 It's normal behavior in Pulsar, Pulsar does not support an "active-standby" consumer across clusters. if you want the consumer can only consume data from one cluster at the same time, you can only start consumers in one cluster, after one cluster not able to provide service, you can switch the consumer to consume data from another cluster(need to enable replicated subscription state, otherwise the consumption state will not sync to another cluster, you might lost data or get many duplicated data from another cluster ).

Thanks @aloyszhang @codelipenghui

authors: aloyszhang casuallc codelipenghui navid1981

repo: apache/pulsar