Horton

Thread

Commits

  1. injection_points: Add tests for name limits

  2. Report better object limits in error messages for injection points

  1. Add tests for object size limits of injection points

    Michael Paquier <michael@paquier.xyz> — 2025-11-10T01:11:42Z

    Hi all,

While looking at a recent patch for injection points that has resulted
in 16a2f706951e, I have been reminded that the point name, library
name and function name have hardcoded limits, and it is now possible
to have them tested by SQL.

Attached is a patch to do so.  Thoughts?
--
Michael

  2. Re: Add tests for object size limits of injection points

    Chao Li <li.evan.chao@gmail.com> — 2025-11-10T02:30:31Z

    
> On Nov 10, 2025, at 09:11, Michael Paquier <michael@paquier.xyz> wrote:
> 
> Hi all,
> 
> While looking at a recent patch for injection points that has resulted
> in 16a2f706951e, I have been reminded that the point name, library
> name and function name have hardcoded limits, and it is now possible
> to have them tested by SQL.
> 
> Attached is a patch to do so.  Thoughts?
> --
> Michael
> <0001-injection_points-Add-tests-for-name-limits.patch>

Maybe not a problem of this patch, but

```
+SELECT injection_points_attach(repeat('a', 64), 'injection_notice',
+  'TestInjectionNoticeFunc', NULL);
+ERROR:  injection point name aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa too long (maximum of 64)
```

Is really confused. The error message says “maximum of 64”, but the test right uses a name of length 64. I know that the tricky is the ‘\0’ terminator, but should SQL writer have to keep mind about the ‘\0’ terminator? Should they just consider maximum length as 63?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  3. Re: Add tests for object size limits of injection points

    Michael Paquier <michael@paquier.xyz> — 2025-11-10T03:31:50Z

    On Mon, Nov 10, 2025 at 10:30:31AM +0800, Chao Li wrote:
> Is really confused. The error message says “maximum of 64”, but the
> test right uses a name of length 64. I know that the tricky is the
> ‘\0’ terminator, but should SQL writer have to keep mind about the
> ‘\0’ terminator? Should they just consider maximum length as 63?

Right.  We could add a "- 1" to the error message printed.
--
Michael

  4. Re: Add tests for object size limits of injection points

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-11-10T10:27:41Z

    Hi Michael, Chao,

On Mon, Nov 10, 2025 at 11:32 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Nov 10, 2025 at 10:30:31AM +0800, Chao Li wrote:
> > Is really confused. The error message says “maximum of 64”, but the
> > test right uses a name of length 64. I know that the tricky is the
> > ‘\0’ terminator, but should SQL writer have to keep mind about the
> > ‘\0’ terminator? Should they just consider maximum length as 63?
>
> Right.  We could add a "- 1" to the error message printed.

Thanks for the patch. I also agree with Chao's suggestion that the
error message better reflects the actual character limits. I
implemented a patch for that and updated the test patch as well.
Please check.

Best,
Xuneng

  5. Re: Add tests for object size limits of injection points

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-10T14:14:02Z

    > On 10 Nov 2025, at 02:11, Michael Paquier <michael@paquier.xyz> wrote:

> While looking at a recent patch for injection points that has resulted
> in 16a2f706951e, I have been reminded that the point name, library
> name and function name have hardcoded limits, and it is now possible
> to have them tested by SQL.

While they do increase code coverage, which is a good thing, it seems somewhat
unlikely that they will catch bugs given the nature of of the code being
tested.  That being said they come at a low cost so no objections to add.

--
Daniel Gustafsson

  6. Re: Add tests for object size limits of injection points

    Michael Paquier <michael@paquier.xyz> — 2025-11-10T23:34:03Z

    On Mon, Nov 10, 2025 at 06:27:41PM +0800, Xuneng Zhou wrote:
> Thanks for the patch. I also agree with Chao's suggestion that the
> error message better reflects the actual character limits. I
> implemented a patch for that and updated the test patch as well.
> Please check.

Yes, that works here.
--
Michael

  7. Re: Add tests for object size limits of injection points

    Michael Paquier <michael@paquier.xyz> — 2025-11-12T01:36:54Z

    On Mon, Nov 10, 2025 at 06:27:41PM +0800, Xuneng Zhou wrote:
> Thanks for the patch. I also agree with Chao's suggestion that the
> error message better reflects the actual character limits. I
> implemented a patch for that and updated the test patch as well.
> Please check.

The restriction related to the private area can be enlarged by 1 byte,
because we don't care about the null-termination in this case.

Done that, and adjusted the whole down to 17 where this has been
introduced.  It's far from being critical, but consistency could
matter for future changes, including things outside of core.  And it's
nice to keep the same rules across the board if we can do so for
testing facilities.
--
Michael

  8. Re: Add tests for object size limits of injection points

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-11-12T02:14:16Z

    Hi,

On Wed, Nov 12, 2025 at 9:37 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Nov 10, 2025 at 06:27:41PM +0800, Xuneng Zhou wrote:
> > Thanks for the patch. I also agree with Chao's suggestion that the
> > error message better reflects the actual character limits. I
> > implemented a patch for that and updated the test patch as well.
> > Please check.
>
> The restriction related to the private area can be enlarged by 1 byte,
> because we don't care about the null-termination in this case.

Makes sense to me.

> Done that, and adjusted the whole down to 17 where this has been
> introduced.  It's far from being critical, but consistency could
> matter for future changes, including things outside of core.  And it's
> nice to keep the same rules across the board if we can do so for
> testing facilities.
> --
> Michael

Thanks.

-- 
Best,
Xuneng

  9. Re: Add tests for object size limits of injection points

    Chao Li <li.evan.chao@gmail.com> — 2025-11-12T02:35:26Z

    
> On Nov 10, 2025, at 18:27, Xuneng Zhou <xunengzhou@gmail.com> wrote:
> 
> Hi Michael, Chao,
> 
> On Mon, Nov 10, 2025 at 11:32 AM Michael Paquier <michael@paquier.xyz> wrote:
>> 
>> On Mon, Nov 10, 2025 at 10:30:31AM +0800, Chao Li wrote:
>>> Is really confused. The error message says “maximum of 64”, but the
>>> test right uses a name of length 64. I know that the tricky is the
>>> ‘\0’ terminator, but should SQL writer have to keep mind about the
>>> ‘\0’ terminator? Should they just consider maximum length as 63?
>> 
>> Right.  We could add a "- 1" to the error message printed.
> 
> Thanks for the patch. I also agree with Chao's suggestion that the
> error message better reflects the actual character limits. I
> implemented a patch for that and updated the test patch as well.
> Please check.
> 
> Best,
> Xuneng
> <v2-0001-injection_points-Report-actual-character-limits-i.patch><v2-0002-injection_points-Add-tests-for-name-limits.patch>

The patch looks good to me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/