Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get-JobList Filter Non-asterisk Wildcard Character Handling #9600

Open
mattcargile opened this issue Feb 24, 2025 · 19 comments
Open

Get-JobList Filter Non-asterisk Wildcard Character Handling #9600

mattcargile opened this issue Feb 24, 2025 · 19 comments
Labels

Comments

@mattcargile
Copy link
Contributor

mattcargile commented Feb 24, 2025

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

If users, use any other wildcard character aside from asterisk, there will be unexpected behavior and the pattern matching will switch to -eq or -ne instead of -like or -notlike. There is also the issue of jobs with an asterisk in their name.

Steps to Reproduce

$sqli = 'sqli'
New-DbaAgentJob -SqlInstance  $sqli -Job 'Filter'
Find-DbaAgentJob -SqlInstance $sqli -JobName filte?

Please confirm that you are running the most recent version of dbatools

2.1.28 

( Version not as relevant as this issue exists in the latest commit on the development branch as of this message )

Other details or mentions

Consider the below class and method instead.

[WildcardPattern]::ContainsWildcardCharacters( 'hi`?')

Use in the below code paths for $jFilter and $sFilter.

foreach ($jFilter in $JobFilter) {
if ($jFilter -match '`*') {
if ($Not) {
$job | Where-Object Name -NotLike $jFilter
} else {
$job | Where-Object Name -Like $jFilter
}
} else {

foreach ($sFilter in $StepFilter) {
if ($sFilter -match '`*') {
if ($Not) {
$stepFound = $job.JobSteps | Where-Object Name -NotLike $sFilter
if ($stepFound.Count -gt 0) {
$job
}
} else {

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

Name                           Value
----                           -----
PSVersion                      7.5.0
PSEdition                      Core
GitCommitId                    7.5.0
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

SQL Server Edition and Build number

Microsoft SQL Server 2017 (RTM-CU20) (KB4541283) - 14.0.3294.2 (X64) 
	Mar 13 2020 14:53:45 
	Copyright (C) 2017 Microsoft Corporation
	Standard Edition (64-bit) on Windows Server 2012 R2 Standard 6.3 <X64> (Build 9600: ) (Hypervisor)

.NET Framework Version

.NET 9.0.1
@mattcargile mattcargile added bugs life triage required New issue that has not been reviewed by maintainers labels Feb 24, 2025
@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

I'd ask why on #9583 this didn't pop up but frankly I think that "featuring" this feature is out of scope.

Either jobname includes SIMPLE pattern matching (aka asterisks) to make things easier for the people (as in quick-and-dirty), or another parameter needs to pop up which accepts a full-scale regex string that gets passed down, because then the issue would be "what if I have a job that has [\d] in it's name", etc ... a proper never-ending story.
At that level (supporting full-scale regexes), I don't see dbatools adding any value. It's not quick, it's not dirty, it's not useful.

Right now the issue arises only for people having asterisks in their job names that can be .... directed elsewhere.

@mattcargile
Copy link
Contributor Author

mattcargile commented Feb 24, 2025

This isn't about [regex]. This is about supporting all the features of -like. Additionally, what if someone has an asterisk in their job name and want to search for it directly. In this implementation, this won't work. [WildcardPattern] is the library backing -like features. I think the behavior is confusing at its current state. All I'm saying is to replace the -match on asterisk with the ContainsWildcardCharaters method. If you really wanted to keep its current state, I would ask to not match on a backtick followed by an asterisk as that assumes the user wants the exact value. I think that just makes the code more complicated than just switching to using the aforementioned method.

@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

ahem, maybe I'm missing something, but it's not as easy as you'd like to be .... what about this ?

[WildcardPattern]::ContainsWildcardCharacters('ab[cde') <-- True

$name = 'abcde'
$name -like 'ab[cde' <-- not valid

which is to say: one thing is -match ("full regex"), another, totally different, is -like

@mattcargile
Copy link
Contributor Author

mattcargile commented Feb 24, 2025

Yeah it should throw on that. But yeah it is a bit more complicated with the error handling. What is worse, is us having some obscure matching syntax with its own quirks. Either we fully support -like or not.

Every command in pwsh should throw especially when they have or evoke a [SupportsWildcards()] attribute. This throws too. Get-Process 'ac['.

Additionally, the docs should be updated in any case.

.PARAMETER JobName
Filter agent jobs to only the name(s) you list.
Supports regular expression (e.g. MyJob*) being passed in.

If you do agree with me in the end, we should add the [SupportsWildcards()] attribute to -JobName on Find-DbaAgentJob.

@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

I concur docs should reflect what we support.
I concur quick and dirty saves the day.
I don't agree the solution is [WildcardPattern]::ContainsWildcardCharacters
Dunno (meaning, we need to check what other functions a-la Get-Process do) if we wanna go the -like route or the -match route, because they're different, and I feel most dbatools users may need the -like more than the -match.

@mattcargile
Copy link
Contributor Author

mattcargile commented Feb 24, 2025

Implementation of -like in C# is done with [WildcardPattern] library. Here is the Get-Process and Get-Service using similar methods.
https://github.com/PowerShell/PowerShell/blob/c6594c2ecc9545efc580da7575c08a3313195625/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs#L173-L208
https://github.com/PowerShell/PowerShell/blob/c6594c2ecc9545efc580da7575c08a3313195625/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs#L372-L425

I agree that most users want -like behavior. If we want -match, then -Pattern makes sense like Find-DbaDbStoredProcedure.

I don't understand why we wouldn't just support the built-in comparisons and use the attributes to mark the support. We should use the wildcard method and if the pattern is wrong then we move to the next one with the same exception handling we use on other commands. Having a non-traditional -like behavior is too confusing. And having to parse the users input to make sure they aren't intentionally searching for an asterisk is more complicated [regex] with multi branching logic checking for both asterisks and /or backticks followed by asterisks. We would be partially implementing ContainsWildcardCharacters ourselves in code at that point.

@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

ok, so let's establish two lines of things.

One is "what we want to support", the other is "how can we support it". Also, we don't want to break existing scripts.

Wanna go for "the way Get-DbaProcess" does (in c#), it probably involves decorating the parameter with something to detect errors in the pattern (didn't check, prolly SupportsWildcards() does it automatically), then the implementation needs to match:

  • 100% sure implementation using [WildcardPattern] matching, using wildcard.IsMatch against the string representation of the job name (100% just because it'd be the exact same done in c#)
  • 80%ish sure implementation of -like (needs to be checked if it results in the very same thing on PS's side)

Then we can document that -JobName accepts wildcards the way -like does, assuming SupportsWildcards() errors out when passing something like ac[ which is an invalid pattern.

If not, [WildcardPattern]::ContainsWildcardCharacters doesn't suffice, and we must find a way to "error out" if the passed pattern can't result in a usable -like.

@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

also, just to not forget, it's going to be ALWAYS a match by -like/-notlike and no more a match via -eq or -ne

@mattcargile
Copy link
Contributor Author

So first off, I don't know why we even have branching logic for -like vs -eq. -like works the same as -eq without wildcards anyway. So I could see dropping the whole logic flow.

Assuming there is something I missed from the above, here is the -like operator implementation using WildcardPattern. There isn't any docs to say this explicitly. It is inferred with the docs here. Then there is the whole implementation going back to v3 which we don't know.

The attribute is only to alert the user for visibility. It doesn't appear to have any teeth with errors. AFAIK, it just sets the help section, supports wild cards, to true.

From the stack trace, we would have to just try-it-out when performing -like and then issue the warning error. This is based on the stack trace from Get-Process and Get-Service.

   at System.Management.Automation.WildcardPatternParser.Parse(WildcardPattern pattern, WildcardPatternParser parser)
   at System.Management.Automation.WildcardPatternMatcher.MyWildcardPatternParser.Parse(WildcardPattern pattern, CharacterNormalizer characterNormalizer)
   at System.Management.Automation.WildcardPatternMatcher..ctor(WildcardPattern wildcardPattern)
   at System.Management.Automation.WildcardPattern.Init()
   at System.Management.Automation.WildcardPattern.IsMatch(String input)
   at Microsoft.PowerShell.Commands.ProcessBaseCommand.RetrieveMatchingProcessesByProcessName()
   at Microsoft.PowerShell.Commands.ProcessBaseCommand.MatchingProcesses()
   at Microsoft.PowerShell.Commands.GetProcessCommand.ProcessRecord()
   at System.Management.Automation.CommandProcessor.ProcessRecord()

Here is a demo function.

Install-PSResource PSFramework
New-Item t.psm1 -conf -valu @'
function Get-SupportsWildcards {
    [CmdletBinding()]
    param (
        [SupportsWildcards()]
        [string]$Value
    )
    
    process {
        try {
            'abc' -like $Value
        }
        catch {
            Stop-PSFFunction -Level Error -Message 'Failed like operator'
            return
        }
    }
}
'@
Import-Module t.psm1 -force
Get-SupportsWildcards 'ab['
Get-Help Get-SupportsWildcards -Parameter Value
Remove-Module t.psm1
Remove_Item t.psm1 -conf

It looks like only Find-DbaAgentJob is using this function now too.

also, just to not forget, it's going to be ALWAYS a match by -like/-notlike and no more a match via -eq or -ne

I'm not sure what you mean by always be a match by -like? Are you alluding to my first comment in this message?

@niphlod
Copy link
Contributor

niphlod commented Feb 24, 2025

yeah, maybe for this exchange slack could have been better, sometimes I prepared replies and altered them on the fly seeing your response coming in.

As for "why do we do -eq/-ne vs -like/-notlike" is probably because who implemented it originally thought about "wildcards" as "asterisks".
The original issue reported [WildcardPattern]::ContainsWildcardCharacters( 'hi?')` as the solution to detect if we wanna do -like or -eq. This doesn't work.

But this issue became later "I want the behaviour to match -like because the current one is non-standard". On this (non-standardi-ness), you have my vote.
This means that "to be standard" we don't do fancy "-eq or -like" but just plain -like.

This CHANGES the behaviour. Repeating myself, while now a "-like" is done if "asterisk" is in the JobName, and -eq in other cases, the new behaviour, if we want this to match a "standard" behaviour, will always be done via "-like". No one can ever ask again to have a way to return a job named myjob and myjab excluding myj*b with a single JobName passed down. -JobName 'myj*b' will match all three of them, we'll use -like matching, standardiness reached, end of story. Wanna pick myj*b ? Need to pass -JobName 'myj`*b'

Back to us, forget about -eq, switch to -like, always. Good. But, we can't accept any string for -like, as some string do not end up being a usable -like.

Also, we're assuming we wanna mirror Get-DbaProcess, which implements via c# (wildcard.isMatch) rather than pure-PS (-like ?).
Notably, this is the help I get for Get-DbaProcess

 -Name <string[]>

        Required?                    false
        Position?                    0
        Accept pipeline input?       true (ByPropertyName)
        Parameter set name           Name, NameWithUserName
        Aliases                      ProcessName
        Dynamic?                     false
        Accept wildcard characters?  false <---

so we're in "mix and match" mode ;-) .

That being said, although strictly undocumented as accepting wildcards, it works, at least in this simple example

PS C:\> Get-Process | where name -like 'powertoys*'

 NPM(K)    PM(M)      WS(M)     CPU(s)      Id  SI ProcessName
 ------    -----      -----     ------      --  -- -----------
     34    11.01      34.80      59.19   14932   3 PowerToys
     67    29.41       5.66       0.80   26500   3 PowerToys.AdvancedPaste
     12    17.91       3.38      28.61   22284   3 PowerToys.AlwaysOnTop
     43    12.25      15.87       2.30   19820   3 PowerToys.Awake
     11    17.56       1.68       0.05   28716   3 PowerToys.CropAndLock
     21    55.99      17.86      19.38    3284   3 PowerToys.FancyZones
     60    27.76       5.65       0.59   24560   3 PowerToys.Peek.UI
    127   366.33      35.85      37.80   13912   3 PowerToys.PowerLauncher

PS C:\> Get-Process 'powertoys*'

 NPM(K)    PM(M)      WS(M)     CPU(s)      Id  SI ProcessName
 ------    -----      -----     ------      --  -- -----------
     34    11.01      34.80      59.19   14932   3 PowerToys
     67    29.41       5.66       0.80   26500   3 PowerToys.AdvancedPaste
     12    17.91       3.38      28.61   22284   3 PowerToys.AlwaysOnTop
     43    12.21      15.85       2.30   19820   3 PowerToys.Awake
     11    17.56       1.68       0.05   28716   3 PowerToys.CropAndLock
     21    55.99      17.85      19.38    3284   3 PowerToys.FancyZones
     60    27.76       5.65       0.59   24560   3 PowerToys.Peek.UI
    127   366.33      35.85      37.80   13912   3 PowerToys.PowerLauncher

Back to the drawing board, if [SupportsWildcards()] is "informational" and doesn't validate, a preprocessor (via try/catch) needs to be put in place to detect invalid -like patterns.

@mattcargile
Copy link
Contributor Author

We are in agreement. I was wrong on a few things and I'm starting to see more of the challenge now.

First off, it is a feature. Sorry to trivialize it and thanks for the patience. :-)

It finally clicked when I considered the below example returning $false. Then the -eq logic would fire and it would a bug.

[wildcardpattern]::ContainsWildcardCharacters( 'hi`*')

And previously I didn't realize the below example would be ok. The logic would use -like and you would be able to match an actual asterisk.

'hi`*' -match '`*'

So, I'm back to square one on how to handle it. I hadn't considered the "Exclude" logic either. I'm mostly of the opinion that -like should be the defacto comparison operator for most of the parameters.

@niphlod
Copy link
Contributor

niphlod commented Feb 25, 2025

I'm not in favour of defaulting everywhere to -like, jobs are usually named with prefixes/common names so this is a good usecase, but e.g. I wouldn't for Get-DbaDatabase.

That being said I think it's time for a good poll to know how people think about this (specifically, jobs, and specifically, supporting the full breadth of wildcards instead of just "asterisks").
Once the path is clear and nobody has valid cons for defaulting to supporting wildcards, the implementation is not as easy as you originally suggested but it's technically doable.

@niphlod niphlod added feature and removed bugs life triage required New issue that has not been reviewed by maintainers labels Feb 25, 2025
@mattcargile
Copy link
Contributor Author

Ok cool, we can focus on just the jobs here maybe? The greater convo about wildcards and -like in the module can be discussed here?

@niphlod
Copy link
Contributor

niphlod commented Feb 26, 2025

sure, don't see a great traction for it but, again, I'm not against "per se".
So, for the Jobs, what's the feature design ?

@mattcargile
Copy link
Contributor Author

I work interactively and I add aliases to dbatools. Sometimes I use the module more than SSMS. Therefore, I enjoy the quick wildcard searches.

Anyway, for Find-DbaAgentJob, I'd like to see -like used on -JobName and -ExcludeJobName. Then we use the [SupportsWildcards()] attribute. If both are used, then maybe we have some logic to warn the user if they might exclude certain jobs based on their conflicting filter. I'd want the fully flexibility of -like on both parameters even at the risk of them colliding and returning $null.

So in the $ExcludeJobName if{}, we could Write-Message -Level Warning if the $output is empty`.

This CHANGES the behaviour. Repeating myself, while now a "-like" is done if "asterisk" is in the JobName, and -eq in other cases, the new behaviour, if we want this to match a "standard" behaviour, will always be done via "-like". No one can ever ask again to have a way to return a job named myjob and myjab excluding myjb with a single JobName passed down. -JobName 'myjb' will match all three of them, we'll use -like matching, standardiness reached, end of story. Wanna pick myj*b ? Need to pass -JobName 'myj`*b'

This bit above I'm still thinking through. I'm pretty sure this would be ok as I think there is enough thrust in PowerShell itself of how wild cards work. I think users were always having to pass a backtick to escape an asterisk on Find-DbaAgentJob.

Implementation

So then this out if checking for asterisks goes away and we use -like exclusively. The below is for the job filter but the step filter should be the same.

if ($jFilter -match '`*') {
if ($Not) {
$job | Where-Object Name -NotLike $jFilter
} else {
$job | Where-Object Name -Like $jFilter
}
} else {
if ($Not) {
$job | Where-Object Name -NE $jFilter
} else {
$job | Where-Object Name -EQ $jFilter
}
}
}

Then the -ExcludeJobName would need to be converted from a -contains to a looped -like.
if ($ExcludeJobName) {
Write-Message -Level Verbose -Message "Excluding job/s based on Exclude"
$output = $output | Where-Object { $ExcludeJobName -notcontains $_.Name }
}

@mattcargile
Copy link
Contributor Author

mattcargile commented Feb 26, 2025

And sorry I'm streaming more thoughts in. Stepping back from this command, is Find-DbaAgentJob and Get-DbaAgentJob all that different? I think there should be a discussion on what Find-Dba does and what Get-Dba does. Personally, I think the Get should be -like and then Find can use -match or other search types.

Sorry the aforementioned issue is kind of bleeding into this one.

@niphlod
Copy link
Contributor

niphlod commented Feb 26, 2025

Giving more thought(s), Find-Dba* COULD support wildcards, Get-Dba* not.
It's kind of the dicotomy of -Path and -LiteralPath of Get-ChildItem.

@mattcargile
Copy link
Contributor Author

mattcargile commented Feb 26, 2025

Yeah Shawn suggested this pattern back in 2023. I'm more against that. I would want Get-Dba* to support wildcards by default like most of the standard cmdlets in PowerShell. I would expect features like Get-DbaAgentJob -ExcludeDisabledJobs to be on Find-DbaAgentJob as a counter point to the differences. Or maybe other non-standard states like an enabled job without a schedule would be a good example of the Find-Dba* feature ( e.g. Find-DbaAgentJob -IsNotScheduled ). Definitely, Find-DbaAgentJob -Pattern with -match support I think too.

@niphlod
Copy link
Contributor

niphlod commented Feb 27, 2025

I'd say 1 vote for the current pattern ("fancy searches via Find-, not Get-") from shawn vs 1 vote to change behaviour from you makes currentpattern+shawn win.

Granted, "fancy" in here ATM is "support asterisks" and we (me and you) would like full-breadth for PS wildcards, which are a SUBSET of regex.

Let's then fix this issue on the perimeter "let's fix Find-DbaAgentJob" and leave the rest up for discussion when the support behind the push to change the current pattern ("fancy searches via Find-, not Get-") is at least 20ish users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants